class object assignment - c++

Question

I have a class 'Record' as follows:

class Record {

private:
    char *bits;
    char* GetBits ();
    void SetBits (char *bits);
    void CopyBits(char *bits, int b_len);

public:
    Record ();
    ~Record();
}

I have an array of Record objects as follows:

Record *recBuffer = new (std::nothrow)Record[RECORD_SIZE];

Assume that the recBuffer has been filled with values. When I print them, they are fine. What could be the problem with the following code?

 Record *newPtr = new (std::nothrow) Record[2*RECORD_SIZE];
 if (NULL == newPtr) {
     return;
 }

 //copy everything
 for (int i = 0; i < RECORD_SIZE; i++) {
     newPtr[i] = recBuffer[i];
 }

 delete[] recBuffer;
 recBuffer = newPtr;

When I try printing the values of recBuffer till RECORD_SIZE, the first few values are corrupted and then there is a segfault finally!

So, I am updating the post with an answer by Ben: So, if you copy a Record (copying the bits pointer), (newPtr[i] = recBuffer[i]) they both will call delete[] with the same pointer. See a problem?

Who owns the buffer pointed-to by `Record::bits`? Most likely you are keeping a pointer after the buffer is freed. — Ben Voigt, Feb 20 '12 at 04:28
Show the implementation of the `Record` member functions, especially including the constructor and destructor. — Ben Voigt, Feb 20 '12 at 04:46
@BenjaminLindley: we are trying to build a database. we have some classes given to us, on which we need to build further. it might be broken, or not a very good class but the functionality provided is very useful and speeds up the work. — aakash, Feb 20 '12 at 05:14
Ridiculous. Whoever wrote the above class could not possibly have produced anything useful. — Benjamin Lindley, Feb 20 '12 at 05:22
@BenjaminLindley: That's not true, they might have plenty of domain knowledge. What is true is that they certainly weren't knowledgeable about C++. — Ben Voigt, Feb 20 '12 at 05:34

score 2 · Accepted Answer · edited May 23 '17 at 11:56

2

You're violating the "Rule of Five": If you have any of the following, you probably need to write all the other four:

user-defined copy constructor
user-defined move constructor
user-defined assignment operator
user-defined move assignment operator
user-defined destructor

See:

Dynamic arrays are used so much that someone has already done all this hard work. Just change bits to a std::vector<char> and all the compiler-generated special member functions will just do the right thing. If bits points to memory owned by the Record object. If you're intending to share memory between many objects, you can consider shared_ptr as suggested by Seth in the comments. But I suspect you aren't wanting any sharing.

edited May 23 '17 at 11:56

Community

1
1

answered Feb 20 '12 at 04:29

Ben Voigt

277,958
43
419
720

Or use `std::shared_ptr` if you want to avoid all that – Seth Carnegie Feb 20 '12 at 04:31
1

@Seth: Better yet, `std::vector`. – Ben Voigt Feb 20 '12 at 04:36
@benvoigt: i am new to c++. do you mean i need to overload the '=' operator? also i thought when we do objA = objB, everything is copied (here the bits pointer will be copied). Is that correct? – aakash Feb 20 '12 at 04:39
@BenVoigt: Record class is a library to me and hence I cannot change it! – aakash Feb 20 '12 at 04:42
1

@Seth: No, I mean `class Record { std::vector bits; /* ... */ };` A non-copyable, non-moveable object can't be contained in `std::vector`, and `Record` seems like it doesn't implement those properly. – Ben Voigt Feb 20 '12 at 04:42
1

@aakash: Show the constructor and destructor bodies then. But it may be so bad you need to throw it away completely. And you're right that `objA = objB` will copy everything including the `bits` pointer... after that you have two `bits` pointers pointing to the same memory. – Ben Voigt Feb 20 '12 at 04:43
@BenVoigt: the constructor and destructor are like this. Record :: Record () { bits = NULL; } Record :: ~Record () { if (bits != NULL) { delete [] bits; } bits = NULL; } – aakash Feb 20 '12 at 04:49
@aakash: So, if you copy a `Record` (copying the `bits` pointer), they both will call `delete[]` with the same pointer. See a problem? – Ben Voigt Feb 20 '12 at 04:52
@BenVoigt: oh!!! damn! yes i got the problem. thanks! i will overload the '=' operator then to solve this problem! thanks to you too, Seth! – aakash Feb 20 '12 at 04:57
1

@aakash: You can't overload `operator=` outside the class, it MUST be a member function. And you said you can't change `class Record`... – Ben Voigt Feb 20 '12 at 04:59
oooh, as i mentioned earlier, sorry i am new to c++ (and OOPS)! any ideas how can resize the array, since overloading cannot be done? – aakash Feb 20 '12 at 05:03
1

@aakash: I think you're stuck using `GetBits` on the old copy, and `SetBits` on the new one. This `class Record` library is really horrible, and if there's any way you can get rid of it, I would. – Ben Voigt Feb 20 '12 at 05:06
@BenVoigt: there are actually 6-7 more functions of the Record class which are extremely useful. No problem, I will use GetBits() and SetBits() as suggested by you. I am not concerned about efficiency right now. thanks a lot everyone! – aakash Feb 20 '12 at 05:10
Oh, now I see that Seth was suggest that you hold `Record` via a pointer. That would allow you to move it from one vector to another without actually copying it. Pretty good idea that. – Ben Voigt Feb 20 '12 at 05:35

class object assignment - c++

1 Answers1