By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
443,433 Members | 771 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 443,433 IT Pros & Developers. It's quick & easy.

A problem when using a reference to a vector of a class with a pointer member to an array

P: n/a
Dear Programmers,

I have a class with a pointer to an array. In the destructor, I just
freed this pointer. A problem happens if I define a reference to a
vector of this kind of class. The destruction of the assigned memory
seems to call the class destructor more than once. I don't know the
reason or whether I used the vector class correctly. Attached is my
program. Thanks for your help.

Regards,
Brian

Run Result:
$ ./a.out
done
*** glibc detected *** double free or corruption (top): 0x08fa40a0 ***
Aborted

Code:
#include <iostream>
#include <vector>

using namespace std;

class ArrayWrap
{
public:
char * pt_ch;
ArrayWrap()
{
pt_ch = new char[100];
}
~ArrayWrap() {
delete[] pt_ch; // if delete this statement, no error then,
// but not a reasonable design
}
};

int main()
{
ArrayWrap dum;
ArrayWrap & rdum = dum;
vector<ArrayWrapdummy(10);
vector<ArrayWrap& refdummy = dummy;

cout << "done" << endl;
return 0;
}

Oct 19 '06 #1
Share this Question
Share on Google+
11 Replies


P: n/a
Brian wrote:
Dear Programmers,

I have a class with a pointer to an array. In the destructor, I just
freed this pointer. A problem happens if I define a reference to a
vector of this kind of class. The destruction of the assigned memory
seems to call the class destructor more than once. I don't know the
reason or whether I used the vector class correctly. Attached is my
program. Thanks for your help.

Regards,
Brian

Run Result:
$ ./a.out
done
*** glibc detected *** double free or corruption (top): 0x08fa40a0 ***
Aborted

Code:
#include <iostream>
#include <vector>

using namespace std;

class ArrayWrap
{
public:
char * pt_ch;
ArrayWrap()
{
pt_ch = new char[100];
}
~ArrayWrap() {
delete[] pt_ch; // if delete this statement, no error then,
// but not a reasonable design
}
};

int main()
{
ArrayWrap dum;
ArrayWrap & rdum = dum;
vector<ArrayWrapdummy(10);
vector<ArrayWrap& refdummy = dummy;

cout << "done" << endl;
return 0;
}
Your wrapper class needs a copy constructor. See the third bullet about
the law of the big three here:

http://www.parashift.com/c++-faq-lit...html#faq-27.10

But, why not just use vector< vector<char or vector<string(see
http://www.parashift.com/c++-faq-lit...tml#faq-34.1)?

Cheers! --M

Oct 19 '06 #2

P: n/a
Thanks a lot. It works. This is a simplified program from my real
work to reproduce the problem. We can't use vector< vector<char or
vector<stringbecause we need to write what the pointer points to as a
whole body to a tape in one unbuffered write() for speed concern. I
don't know any better way.

However, I still don't understand the reason for the need of copy
constructor. During the lunch, I came out of the idea that why I don't
just use a pointer to the established vector in stead of a reference.
To my surprise, it doesn't work either without a copy constructor. I
don't get it. I didn't make any copy, but just made a pointer or a
reference referring to the established vector object. I don't see
where a copy was made. I didn't put anything in the copy constructor
and it still works. Is this a requirement that the standard library
has? Are there some hidden copy operations made when we define a
vector reference or pointer?

Thanks,
Brian

mlimber wrote:
Your wrapper class needs a copy constructor. See the third bullet about
the law of the big three here:

http://www.parashift.com/c++-faq-lit...html#faq-27.10

But, why not just use vector< vector<char or vector<string(see
http://www.parashift.com/c++-faq-lit...tml#faq-34.1)?

Cheers! --M
Oct 19 '06 #3

P: n/a
"Brian" <su******@myrealbox.comwrote in message
news:11**********************@b28g2000cwb.googlegr oups.com...
Thanks a lot. It works. This is a simplified program from my real
work to reproduce the problem. We can't use vector< vector<char or
vector<stringbecause we need to write what the pointer points to as a
whole body to a tape in one unbuffered write() for speed concern. I
don't know any better way.

However, I still don't understand the reason for the need of copy
constructor. During the lunch, I came out of the idea that why I don't
just use a pointer to the established vector in stead of a reference.
To my surprise, it doesn't work either without a copy constructor. I
don't get it. I didn't make any copy, but just made a pointer or a
reference referring to the established vector object. I don't see
where a copy was made. I didn't put anything in the copy constructor
and it still works. Is this a requirement that the standard library
has? Are there some hidden copy operations made when we define a
vector reference or pointer?

Thanks,
Brian

mlimber wrote:
>Your wrapper class needs a copy constructor. See the third bullet about
the law of the big three here:

http://www.parashift.com/c++-faq-lit...html#faq-27.10

But, why not just use vector< vector<char or vector<string(see
http://www.parashift.com/c++-faq-lit...tml#faq-34.1)?

Cheers! --M
Please don't top post. Please supply more context from previous post.

int main()
{
ArrayWrap dum;
ArrayWrap & rdum = dum;
vector<ArrayWrapdummy(10);
// I believe the previous line is your problem

vector<ArrayWrap& refdummy = dummy;

cout << "done" << endl;
return 0;
}

Vectors tend to be a pain this way. It seems (to me) that when you put an
item into a vector it first creates it into a temporary, then copies the
temporary into the vector. So when the temporary gets destroyed, it runs
the destructor and you're hozed.

I've run into this problem before with classes that are not copyable (
unique objects loaded that can't be copied, or are just a waste of time to
copy). The way I've solved this is to make a private copy constructor (so
it *can't* be copied) then to make my vector into pointers.

vector<ArrayWrap*dummy;
for ( i = 0; i < 10; i++ )
ArrayWrap.push_back( new ArrayWrap );
// code

for ( vector<ArrayWrap>::iterator it = ArrayWrap.begin(); it !=
ArrayWrap.end(); ++it )
delete (*it); // double check syntax of this
Oct 19 '06 #4

P: n/a

Jim Langston wrote in message ...
>
vector<ArrayWrap*dummy;
for ( i = 0; i < 10; i++ )
Got a headache today, Jim? Did you mean?

// ArrayWrap.push_back( new ArrayWrap );
dummy.push_back( new ArrayWrap );

// <GBeen there, done that. <G>
>// code
// >for ( vector<ArrayWrap>::iterator it = ArrayWrap.begin(); it !=
// >ArrayWrap.end(); ++it )
for( vector<ArrayWrap*>::iterator it( dummy.begin() ); it !=
dummy.end(); ++it )
delete (*it); // double check syntax of this
Yep, looks same as one I use:
// ------------------------------------
template<class Seqvoid PurgeCon( Seq &cont ){
typename Seq::iterator it;
for( it = cont.begin(); it != cont.end(); ++it ){
delete *it;
*it = 0; // just in case it's called twice.
} // for(it)
} // template<class Seqvoid PurgeCon(Seq&)
// ------------------------------------

PurgeCon( dummy );

--
Bob R
POVrookie
Oct 20 '06 #5

P: n/a
"BobR" <Re***********@worldnet.att.netwrote in message
news:4c*********************@bgtnsc05-news.ops.worldnet.att.net...
>
Jim Langston wrote in message ...
>>
vector<ArrayWrap*dummy;
for ( i = 0; i < 10; i++ )

Got a headache today, Jim? Did you mean?

// ArrayWrap.push_back( new ArrayWrap );
dummy.push_back( new ArrayWrap );

// <GBeen there, done that. <G>
Heh, yeah, nice catch.
>>// code
// >for ( vector<ArrayWrap>::iterator it = ArrayWrap.begin(); it !=
// >ArrayWrap.end(); ++it )
for( vector<ArrayWrap*>::iterator it( dummy.begin() ); it !=
dummy.end(); ++it )
Me and ArrayWrap again instead of dummy. lo.

I notice you use it (dummy.begin()) to construct the iterator. I've never
known I could do this, but does it really save anything?
>
> delete (*it); // double check syntax of this

Yep, looks same as one I use:
// ------------------------------------
template<class Seqvoid PurgeCon( Seq &cont ){
typename Seq::iterator it;
for( it = cont.begin(); it != cont.end(); ++it ){
delete *it;
Yet, here you don't use the initialization, why?
*it = 0; // just in case it's called twice.
} // for(it)
} // template<class Seqvoid PurgeCon(Seq&)
// ------------------------------------

PurgeCon( dummy );

Oct 20 '06 #6

P: n/a

"Brian" <su******@myrealbox.comwrote in message
news:11**********************@b28g2000cwb.googlegr oups.com...
Thanks a lot. It works. This is a simplified program from my real
work to reproduce the problem. We can't use vector< vector<char or
vector<stringbecause we need to write what the pointer points to as a
whole body to a tape in one unbuffered write() for speed concern. I
don't know any better way.
However, I still don't understand the reason for the need of copy
constructor.

Not understanding what you really did (please post your updated class), the
vector requires that your object are copyable, and to make sure no bugs
exist, the objects should be able to be copied safely with no issues. Your
original ArrayWrap class does not satisfy the "safely copyable" requirement
(although it is copyable).

int main()
{
ArrayWrap a;
ArrayWrap b = a;
}

Try this with your original class, or with the class that you have changed
it to. Does this work correctly (i.e. no memory leaks, crashes, double
deletion errors, etc.)? It won't work with your original class, since at
the end of main(), a double deletion error will occur when object a and b
are destroyed.

That's simply what vector is doing, and if there are bugs with the simple
code above, then there will be bugs when vector gets its hands on it.

- Paul

Oct 20 '06 #7

P: n/a

Brian wrote:
Thanks a lot. It works. This is a simplified program from my real
work to reproduce the problem. We can't use vector< vector<char or
vector<stringbecause we need to write what the pointer points to as a
whole body to a tape in one unbuffered write() for speed concern. I
don't know any better way.
When you say "what the pointer points to" are you talking about the
pointer in your ArrayWrap class? If so then what you need does not
preclude you replacing ArrayWrap with vector<charand replacing
vector<ArrayWrapwith vector<vector<char. "What the pointer points
to" in your ArrayWrap class becomes "the contents of a vector<char>".
You can very easily get at the contents of a vector<charfor your
single unbuffered write.

If I have misunderstood you when you say "what the pointer points to"
please clarify as there may still be a better solution.

Gavin Deane

Oct 20 '06 #8

P: n/a

Jim Langston wrote in message ...
>"BobR" wrote in message
>>
Jim Langston wrote in message ...
>>>
vector<ArrayWrap*dummy;
for ( i = 0; i < 10; i++ )

Got a headache today, Jim? Did you mean?
// ArrayWrap.push_back( new ArrayWrap );
dummy.push_back( new ArrayWrap );
// <GBeen there, done that. <G>

Heh, yeah, nice catch.
>>>// code
// >for ( vector<ArrayWrap>::iterator it = ArrayWrap.begin(); it !=
// >ArrayWrap.end(); ++it )
for( vector<ArrayWrap*>::iterator it( dummy.begin() ); it !=
dummy.end(); ++it )

Me and ArrayWrap again instead of dummy. lo.

I notice you use it (dummy.begin()) to construct the iterator. I've never
known I could do this, but does it really save anything?
Sure, if you did it a million times in your program and optimazation was off,
it could save a second in runtime! <GActually, I was just being a
smart-ass.
If you are used to assignment (=), you should be consistant and always use
assignment in your code. Mostly a 'style choice'.

You seldom see:

class Mine{.....};
Mine My = 12345;
.....but, usually:
Mine My(12345);

.....so....

int Num(12345);

.... is my choice of style. <G>
>
>>
>> delete (*it); // double check syntax of this

Yep, looks same as one I use:
// ------------------------------------
template<class Seqvoid PurgeCon( Seq &cont ){
// typename Seq::iterator it;
// for( it = cont.begin(); it != cont.end(); ++it ){

Yet, here you don't use the initialization, why?
Uuuh, old code? Stole the code from someone and didn't change it?

for( typename Seq::iterator it( cont.begin() ); it != cont.end(); ++it){

// typename Seq::iterator it( cont.begin() );
// for( ; it != cont.end(); ++it ){ delete *it; *it = 0; }

Newbies, see:
"Thinking In C++", Volume 2: Practical Programming (Bruce Eckel, Chuck
Allison)
Chap. 5: Templates in Depth (sub "The typename keyword")
http://www.mindview.net/Books/TICPP/...ngInCPP2e.html
.....for why the 'typename' is needed. (or look it up in *your* book)
> delete *it;
*it = 0; // just in case it's called twice.
} // for(it)
} // template<class Seqvoid PurgeCon(Seq&)
// ------------------------------------

PurgeCon( dummy );

[ I'm not a expert, feel free to correct me. ]
--
Bob R
POVrookie
Oct 20 '06 #9

P: n/a
Dear Jim,

Thank you very much. Your explanation is right. It was not the
reference or pointer that caused the problem but the place your pointed
out.

The vector constructor does create a temporary object and copy it to
all the vector members and destroy it. I put some cout statements in
the constructor, copy constructor and destructor and got relevant
information.

I said that when I put nothing in the copy constructor it still works.
I was wrong. It only works when I don't access anything within the
object, otherwise it causes segmentation error because all its members
will have no valid value if nothing was done in the copy constructor.

I attached my updated program below for context and left some comments.
Now I don't need your technique of pointers. I will keep it in mind
in case needed. Thanks a lot.

Regards,
Brian

Jim Langston wrote:
>
Please don't top post. Please supply more context from previous post.

int main()
{
ArrayWrap dum;
ArrayWrap & rdum = dum;
vector<ArrayWrapdummy(10);
// I believe the previous line is your problem

vector<ArrayWrap& refdummy = dummy;

cout << "done" << endl;
return 0;
}

Vectors tend to be a pain this way. It seems (to me) that when you put an
item into a vector it first creates it into a temporary, then copies the
temporary into the vector. So when the temporary gets destroyed, it runs
the destructor and you're hozed.

I've run into this problem before with classes that are not copyable (
unique objects loaded that can't be copied, or are just a waste of time to
copy). The way I've solved this is to make a private copy constructor (so
it *can't* be copied) then to make my vector into pointers.

vector<ArrayWrap*dummy;
for ( i = 0; i < 10; i++ )
ArrayWrap.push_back( new ArrayWrap );
// code

for ( vector<ArrayWrap>::iterator it = ArrayWrap.begin(); it !=
ArrayWrap.end(); ++it )
delete (*it); // double check syntax of this
------------------------------------------------------------------------------------------------------
#include <iostream>
#include <vector>

using namespace std;

class ArrayWrap
{
public:
char * pt_ch;
ArrayWrap()
{
pt_ch = new char[100];
pt_ch[0] = 'b';
cout << "create dummy" << endl;
}

ArrayWrap(const ArrayWrap & AnArray)
{
pt_ch = new char[100]; // this made copy's pt_ch valid
pt_ch[0] = AnArray.pt_ch[0]; // this assigned values to
// the copy.
cout << "copy dummy" << endl;
}

~ArrayWrap()
{
delete[] pt_ch;
cout << "destruct dummy" << endl;
}

int print()
{
cout << pt_ch[0] << endl;
return 1;
}
};

int main()
{
ArrayWrap dum;
vector<ArrayWrapdummy(5); // copy constructor
// is needed here, otherwise
// pt_ch of dummy[i] points
// to destroyed temporary
// ArrayWrap object
vector<ArrayWrap& refdummy = dummy;
vector<ArrayWrap* pdummy;
dummy[0].print(); // this will cause segmentation
// error if copy constructors
// first two statements is omitted
cout << "done" << endl;
return 0;
}

Oct 27 '06 #10

P: n/a

Brian wrote in message ...
>Please don't top post. Please supply more context from previous post.
But, do trim stuff you are not referencing. <G>

>----------------------------------------------------------------------------
--------------------------
>#include <iostream>
#include <vector>
using namespace std;
A logical next step could be to get rid of the "magic numbers".
const int bufsize = 100;
// int const bufsize(100); // another way to write it.

Now, if you decide to change the size of your 'char array', you only need to
change it in one place and it's all taken care of for you. (see changes
below)

>class ArrayWrap{
public:
char * pt_ch;
ArrayWrap(){
// pt_ch = new char[100];
pt_ch = new char[ bufsize ];
pt_ch[0] = 'b';
cout << "create dummy" << endl;
}

ArrayWrap(const ArrayWrap & AnArray){
// pt_ch = new char[100]; // this made copy's pt_ch valid
pt_ch = new char[ bufsize ]; // this made copy's pt_ch valid

pt_ch[0] = AnArray.pt_ch[0]; // this assigned values to
// the copy.
// Since you declared an 'const int' for the buffer size, you could copy
// the whole thing (not just one char) without doing any other 'checks':

for( int i(0); i < bufsize; ++i){ // simple example
pt_ch[ i ] = AnArray.pt_ch[ i ];
} // for(i)
// [ look up 'std::copy', 'memcpy', etc., for other ways. ]
cout << "copy dummy" << endl;
}

~ArrayWrap(){
delete[] pt_ch;
cout << "destruct dummy" << endl;
}

int print()
{
cout << pt_ch[0] << endl;
return 1;
}
};

int main(){
ArrayWrap dum;
If you don't use 'dum', remove that instance. etc..
vector<ArrayWrapdummy(5); // copy constructor
// is needed here, otherwise
// pt_ch of dummy[i] points
// to destroyed temporary
// ArrayWrap object
vector<ArrayWrap& refdummy = dummy;
vector<ArrayWrap* pdummy;
dummy[0].print(); // this will cause segmentation
// error if copy constructors
// first two statements is omitted
cout << "done" << endl;
return 0;
}
--
Bob R
POVrookie
Oct 28 '06 #11

P: n/a
Hi, Gavin,

Sorry for the late reply. I did mean the pointer in my ArrayWrap class
here, which is "pt_ch" in my original program. To my knowledge, we can
only access each member of a vector one by one in stead of a whole
body. How can I use one write to write all values in a vector to a
file?

I attached my old program in case it's lost.

Thanks,
Brian

Gavin Deane wrote:
Brian wrote:
Thanks a lot. It works. This is a simplified program from my real
work to reproduce the problem. We can't use vector< vector<char or
vector<stringbecause we need to write what the pointer points to as a
whole body to a tape in one unbuffered write() for speed concern. I
don't know any better way.

When you say "what the pointer points to" are you talking about the
pointer in your ArrayWrap class? If so then what you need does not
preclude you replacing ArrayWrap with vector<charand replacing
vector<ArrayWrapwith vector<vector<char. "What the pointer points
to" in your ArrayWrap class becomes "the contents of a vector<char>".
You can very easily get at the contents of a vector<charfor your
single unbuffered write.

If I have misunderstood you when you say "what the pointer points to"
please clarify as there may still be a better solution.

Gavin Deane
Code:
#include <iostream>
#include <vector>

using namespace std;

class ArrayWrap
{
public:
char * pt_ch;
ArrayWrap()
{
pt_ch = new char[100];
}
~ArrayWrap() {
delete[] pt_ch; // if delete this statement, no error then,
// but not a reasonable design
}

};

int main()
{
ArrayWrap dum;
ArrayWrap & rdum = dum;
vector<ArrayWrapdummy(10);
vector<ArrayWrap& refdummy = dummy;

cout << "done" << endl;
return 0;
}

Nov 13 '06 #12

This discussion thread is closed

Replies have been disabled for this discussion.