ptr in collection item

[tuvok]

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?

#include <vector>

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor
Item(int AiId) : iId(AiId) { pMiscMem = malloc(1024); }
~Item() { free(pMiscMem), pMiscMem = 0; }
};

class MyColl
{
public:
std::vector<Ite m> v;
MyColl() {}
void Add(Item* pItem) { v.push_back(*pI tem); }
};

void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 0; i < 100; ++i)
C.Add(new Item(i));
}

[Victor Bazarov]

tuvok wrote:

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?

Read about "the Rule of Three".

V

[Jason Heyes]

Your copy constructor (automatically generated by the compiler) does not
allocate new memory. Are you trying to use heterogeneous containers like in
Java? You should avoid this.

[tuvok]

"Victor Bazarov" wrote

tuvok wrote:

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?

Read about "the Rule of Three".

Thank you. I did and came up with the following but it still crashes :-(
What's missing?

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor

Item(int AiId) : iId(AiId), pMiscMem(0)
{ pMiscMem = malloc(1024); }

~Item()
{ free(pMiscMem), pMiscMem = 0; }

Item(const Item& Other)
{
free(pMiscMem);

iId = Other.iId;
pMiscMem = Other.pMiscMem;

Other.~Item();
}

const Item& operator=(const Item& Other)
{
free(pMiscMem);

iId = Other.iId;
pMiscMem = Other.pMiscMem;

Other.~Item();

return *this;
}
};

class MyColl
{
public:
std::vector<Ite m> v;
MyColl() {}
void Add(Item* pItem) { v.push_back(*pI tem); }
};

void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 0; i < 100; ++i)
C.Add(new Item(i));
}

[tuvok]

"Jason Heyes" wrote

Your copy constructor (automatically generated by the compiler) does not
allocate new memory. Are you trying to use heterogeneous containers like in Java?

I think not. Cf. code. The container shall contain objects of the same one type.

[Jason Heyes]

Remove 'free(pMiscMem) ' in your copy constructor and all lines containing
'Other.~Item()' .

[Larry I Smith]

tuvok wrote:

"Victor Bazarov" wrote

tuvok wrote:

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?
Read about "the Rule of Three".

Thank you. I did and came up with the following but it still crashes :-(
What's missing?

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor

Item(int AiId) : iId(AiId), pMiscMem(0)
{ pMiscMem = malloc(1024); }

Try using 'new' and delete[], rather than malloc() and free()?
Why is pMiscMem "void *", what will it hold?

~Item()
{ free(pMiscMem), pMiscMem = 0; }

Item(const Item& Other)
{
free(pMiscMem);

iId = Other.iId; pMiscMem = Other.pMiscMem;

This is a copy constructor, you should be making
a copy of "Other" - not taking over its data.
You should replace the above line with something
like this:

pMiscMem = malloc(1024);
memcpy(pMiscMem , Other.pMiscMem, 1024);

Otherwise you'll have 2 Item objects who's pMiscMem
pointers point to the same memory block. When either
of those Item objects is destructed, the memory that
both Item.pMiscMem members point to will be deleted.
That will leave the pMiscMem of the remaining 'Item'
object pointing to memory that already been freed;
causing a crash when the 2nd Item object is destructed.

Other.~Item();
Don't delete 'Other' in your copy constructor.
The purpose of the copy constructor is to make a copy
of 'Other' - and that's all.

}

const Item& operator=(const Item& Other)
{
free(pMiscMem);

iId = Other.iId; pMiscMem = Other.pMiscMem;
This is an assignmet operator, you should be making
a copy of "Other" - not taking over its data.
You should replace the above line with something
like this:

pMiscMem = malloc(1024);
memcpy(pMiscMem , Other.pMiscMem, 1024);

Otherwise you'll have 2 Item objects who's pMiscMem
pointers point to the same memory block. When either
of those Item objects is destructed, the memory that
both Item.pMiscMem members point to will be deleted.
That will leave the pMiscMem of the remaining 'Item'
object pointing to memory that already been freed;
causing a crash when the 2nd Item object is destructed.

Other.~Item();

Don't delete 'Other' in your assignment operator.
The purpose of the assignment operator is to make a
copy of the data from 'Other' in 'this' - and that's all.


return *this;
}
};

class MyColl
{
public:
std::vector<Ite m> v;
MyColl() {}
void Add(Item* pItem) { v.push_back(*pI tem); }
};

void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 0; i < 100; ++i)
C.Add(new Item(i));
}

Larry

[Larry I Smith]

tuvok wrote:

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?

#include <vector>

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor
Item(int AiId) : iId(AiId) { pMiscMem = malloc(1024); }
~Item() { free(pMiscMem), pMiscMem = 0; }
};

class MyColl
{
public:
std::vector<Ite m> v;
MyColl() {}
void Add(Item* pItem) { v.push_back(*pI tem); }
};

void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 0; i < 100; ++i)
C.Add(new Item(i));
}

There is a memory leak in the line:

C.Add(new Item(i));

You use 'new' to allocate an 'Item'.
A POINTER to that 'Item' is passed to Add() which
calls v.push_back(*pI tem). push_back() makes a
COPY of it's arg (a copy of "*pItem" in this case),
then puts that copy into the vector. The original
'Item' allocated by 'new' is never deleted.
Here's one possible alternative:

void test_ptr_in_vec tor_item()
{
MyColl C;
Item * pItem;

for (int i = 0; i < 100; ++i)
{
pItem = new Item(i);
C.Add(pItem);
delete pItem;
}
}

The name 'test_ptr_in_ve ctor()' is misleading.
You do not put a pointer to 'Item' (an "Item *")
in your vector; you put an actual 'Item'.

Larry

[tuvok]

"Larry I Smith" wrote

tuvok wrote:

"Victor Bazarov" wrote

tuvok wrote:
I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?

Read about "the Rule of Three".

Thank you. I did and came up with the following but it still crashes :-(
What's missing?

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor

Item(int AiId) : iId(AiId), pMiscMem(0)
{ pMiscMem = malloc(1024); }

Try using 'new' and delete[], rather than malloc() and free()?
Why is pMiscMem "void *", what will it hold?

Nothing special. Just a sample for demo purposes.

~Item()
{ free(pMiscMem), pMiscMem = 0; }

Item(const Item& Other)
{
free(pMiscMem);

iId = Other.iId;

pMiscMem = Other.pMiscMem;

This is a copy constructor, you should be making
a copy of "Other" - not taking over its data.
You should replace the above line with something
like this:

pMiscMem = malloc(1024);
memcpy(pMiscMem , Other.pMiscMem, 1024);

Otherwise you'll have 2 Item objects who's pMiscMem
pointers point to the same memory block. When either
of those Item objects is destructed, the memory that
both Item.pMiscMem members point to will be deleted.
That will leave the pMiscMem of the remaining 'Item'
object pointing to memory that already been freed;
causing a crash when the 2nd Item object is destructed.

Ok, got it.
But wouldn't it be faster if 'this' would just take the resources of 'Other'
and clear them in 'Other'? IOW transferring the ownership from
'Other' to 'this'. The following solution does it this way. Is it ok to do it so,
or is there anything against it?

class Item
{
public:
int iId;
void* pMiscMem; // will be allocd in ctor and deallocd in dtor

Item(int AiId) : iId(AiId)
{
pMiscMem = malloc(1024); // just a sample for ptr usage in collection items
}

~Item()
{
if (pMiscMem)
free(pMiscMem);
ClearOwnership( );
}

void ClearOwnership( )
{ // just clears the resources (but does not destroy them!)
// intended especially for ptrs.
// will be called from copy_ctor, assignment_oper ator, and dtor
pMiscMem = 0; // setting ptrs to 0 is important
iId = 0;
}

Item(const Item& Other)
{ // The object will be created (initialized) from the other one (Other),
// Therefore the normal ctor won't be called.
// So nothing is initialized yet!
// The initialization from Other is the job of this copy constructor.

// set resources of this from Other:
iId = Other.iId;
pMiscMem = Other.pMiscMem;

// take ownership by clearing Other:
((Item*) &Other)->ClearOwnership ();
}

const Item& operator=(const Item& Other)
{
// free resources of this:
free(pMiscMem);

// set resources of this from Other:
iId = Other.iId;
pMiscMem = Other.pMiscMem;

// take ownership by clearing Other:
((Item*) &Other)->ClearOwnership ();

return *this;
}
};

class MyColl
{
public:
std::vector<Ite m> v;
MyColl() {}
void Add(Item& rItem) { v.push_back(rIt em); }
};

void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 1; i <= 10000; ++i)
{
Item obj(i);
C.Add(obj);
}
}

[tuvok]

"Larry I Smith" wrote

tuvok wrote:

I have an Item object which allocates memory in its ctor
and frees it in its dtor.
I want to add such items to a vector. But the program crashes. Why?
How do I fix it?
.... void test_ptr_in_vec tor_item()
{
MyColl C;
for (int i = 0; i < 100; ++i)
C.Add(new Item(i));
}

There is a memory leak in the line:

C.Add(new Item(i));

You use 'new' to allocate an 'Item'.
A POINTER to that 'Item' is passed to Add() which
calls v.push_back(*pI tem). push_back() makes a
COPY of it's arg (a copy of "*pItem" in this case),
then puts that copy into the vector. The original
'Item' allocated by 'new' is never deleted.
Here's one possible alternative:

void test_ptr_in_vec tor_item()
{
MyColl C;
Item * pItem;

for (int i = 0; i < 100; ++i)
{
pItem = new Item(i);
C.Add(pItem);
delete pItem;
}
}

Thanks. Got it.
The name 'test_ptr_in_ve ctor()' is misleading.
You do not put a pointer to 'Item' (an "Item *")
in your vector; you put an actual 'Item'.

Yeah. It better should be "test_ptr_in_co llectionitem()"