delete dynamically allocated memory in a list?

[Yi]

Two questions about the following code sample:

--- code begins ---
//class IPv4 is defined elsewhere

list<IPv4> ip_list;

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

//IPv4 new_ip(addr);
//ip_list.push_back(new_ip);
ptr = new IPv4(addr);
ip_list.push_back(*ptr);
}

....

list<IPv4>::iterator k;
k = ip_list.begin();
while (k!=pp_list.end()) {
delete &pp_list.front();
pp_list.pop_front();
k = pp_list.begin();
}

--- code ends ---

When I try to free the memory using delete, the program run into error
saying "double free or corruption". I don't know why. How am I supposed
to delete the dynamically allocated memory in a list?

By the way, if I use the two statement that are currently commented in
the first for loop instead of the two statements below them, I don't
need to worry about freeing the memory, right?

Thanks!

 

[Vikram]

Two questions about the following code sample:

--- code begins ---
//class IPv4 is defined elsewhere

list<IPv4> ip_list;

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

//IPv4 new_ip(addr);
//ip_list.push_back(new_ip);
ptr = new IPv4(addr);
ip_list.push_back(*ptr);

Note that here a *copy* of *ptr is pushed into the list. It is not the
same object.

}

...

list<IPv4>::iterator k;
k = ip_list.begin();
while (k!=pp_list.end()) {
delete &pp_list.front();

This does not fit here. Your list element is an object and not a
pointer. So you should not be deleting it.

pp_list.pop_front();
k = pp_list.begin();
}

--- code ends ---

When I try to free the memory using delete, the program run into error
saying "double free or corruption". I don't know why. How am I supposed
to delete the dynamically allocated memory in a list?

You should do a delete ptr above and not delete a list element. They
are different objects.
Hope this helps.

 

[Yi]

Thanks a lot Vikram!

You should do a delete ptr above and not delete a list element. They
are different objects.

Do you mean the following?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

//IPv4 new_ip(addr);
//pp_list.push_back(new_ip);
ptr = new IPv4(addr);
pp_list.push_back(*ptr);
delete ptr;
}
....

As I asked in the previous post, is it simpler to just use the
following code?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

IPv4 new_ip(addr);
pp_list.push_back(new_ip);
//ptr = new IPv4(addr);
//pp_list.push_back(*ptr);
//delete ptr;
}

Thanks a lot!

 

[Vikram]

You should do a delete ptr above and not delete a list element. They
are different objects.

Do you mean the following?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

//IPv4 new_ip(addr);
//pp_list.push_back(new_ip);
ptr = new IPv4(addr);
pp_list.push_back(*ptr);
delete ptr;
}
...

As I asked in the previous post, is it simpler to just use the
following code?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

IPv4 new_ip(addr);
pp_list.push_back(new_ip);
//ptr = new IPv4(addr);
//pp_list.push_back(*ptr);
//delete ptr;
}

Yes...either of the above is fine. In one case you take care of delete
and in the second (which is more convenient) the local variable cleanup
gets automatically done. I would personally go with the second approach
( new_ip).

 

[Yi]

Yeah, I agree!

And one more dumb question: since the scope of new_ip is within the
pair of { }, there is no problem of repeated definitions among
iterations, right?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

IPv4 new_ip(addr);
pp_list.push_back(new_ip);
//ptr = new IPv4(addr);
//pp_list.push_back(*ptr);
//delete ptr;
}

Yes...either of the above is fine. In one case you take care of delete
and in the second (which is more convenient) the local variable cleanup
gets automatically done. I would personally go with the second approach
( new_ip).

 

[David Harmon]

On 22 Aug 2006 23:09:39 -0700 in comp.lang.c++, "Yi"

And one more dumb question: since the scope of new_ip is within the
pair of { }, there is no problem of repeated definitions among
iterations, right?

Right, it is automatically created and destroyed for each iteration,
but only one at a time.

 

[Michiel.Salters]

As I asked in the previous post, is it simpler to just use the
following code?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

IPv4 new_ip(addr);
pp_list.push_back(new_ip);
//ptr = new IPv4(addr);
//pp_list.push_back(*ptr);
//delete ptr;
}

Why not do it in one line?

pp_list.push_back(addr);

There is probably no need to have new_ip.

HTH,
Michiel Salters

 

[peter koch]

As I asked in the previous post, is it simpler to just use the
following code?

for (int i=1; i<=9; i++) {
char addr[128];
sprintf(addr, "%d.%d.%d.%d", i,i,i,i);

IPv4 new_ip(addr);
pp_list.push_back(new_ip);
//ptr = new IPv4(addr);
//pp_list.push_back(*ptr);
//delete ptr;
}

Why not do it in one line?

pp_list.push_back(addr);

There is probably no need to have new_ip.

If I wrote IPv4 i would probably have made the constructor explicit.
Thus:
pp_list.push_back(IPv4(addr));

/Peter