memory allocation/deallocation question

[BigBrian]

I realize this isn't the best code, but I'm doing maintenance on a
legcy system that has something similar to the following...

class Foo
{
//...
}

int main()
{
char * p_char = new char[5000];

// p_char gets filled with data read from a socket

Foo * p_foo = (Foo *) p_char;

// p_foo gets used here...

delete p_foo;
}

Assuming Foo is smaller than 5000 * sizeof(char), will this result in a
memory leak? Or will the call to delete know that more memory was
allocated than sizeof(Foo), and do the right thing?

 

[Pete Becker]

Assuming Foo is smaller than 5000 * sizeof(char), will this result in a
memory leak? Or will the call to delete know that more memory was
allocated than sizeof(Foo), and do the right thing?

No guarantees either way. Do it like this:

p_foo->~Foo(); // destroy object
delete [] p_char; // free the memory

 

[Andre Kostur]

Assuming Foo is smaller than 5000 * sizeof(char), will this result in a
memory leak? Or will the call to delete know that more memory was
allocated than sizeof(Foo), and do the right thing?

No guarantees either way. Do it like this:

p_foo->~Foo(); // destroy object
delete [] p_char; // free the memory

Even that's not safe. Note that it hasn't been mentioned that *p_foo has
ever been constructed! And, isn't the OP's stuff undefined behaviour
anyway... he's doing a delete on memory that was new[]'ed (ignoring that
he's switching types too...).

 

[Pete Becker]

Even that's not safe. Note that it hasn't been mentioned that *p_foo has
ever been constructed!

"Hasn't been mentioned" is not the same as "hasn't happened." I'm
willing to make the obvious assumption that the data read in produces a
properly initialized object. But if you want to ask him about it, go ahead.

And, isn't the OP's stuff undefined behaviour
anyway... he's doing a delete on memory that was new[]'ed (ignoring that
he's switching types too...).

Could that be what "No guarantees either way" means?

 

[BigBrian]

In this legacy system, the object is correctly created from the char
array by the cast. I realize this isn't portable nor is it the best
way to implement this. However, I can't address these issues at this
time. I was specifically concerned with leaking memory. Operator new
obviously keeps information about how much memory was allocated (
otherwise delete[] p_char wouldn't know how much to delete), but I
didn't know if this was specific to the type of pointer which is
deleted, ie the delete p_foo.

-Brian

 

[Old Wolf]

[stuff snipped in parent posts has been replaced]

BigBrian wrote:

class Foo { ......... };
int main()
{
char * p_char = new char[5000];
// p_char gets filled with data read from a socket
Foo * p_foo = (Foo *) p_char;
delete p_foo;
}

p_foo->~Foo();
delete [] p_foo;

Even that's not safe. Note that it hasn't been mentioned that
*p_foo has ever been constructed!

"Hasn't been mentioned" is not the same as "hasn't happened."
I'm willing to make the obvious assumption that the data read
in produces a properly initialized object.

The behaviour of dereferencing p_foo is undefined if Foo
is not a POD (because non-PODs can only be correctly
constructed by calling their constructor).

So we should assume that Foo is POD. Therefore, calling its
destructor is unnecessary.

 

[Pete Becker]

[stuff snipped in parent posts has been replaced]

[Stuff unnecessarily replaced has been snipped]

The behaviour of dereferencing p_foo is undefined if Foo
is not a POD (because non-PODs can only be correctly
constructed by calling their constructor).

So we should assume that Foo is POD. Therefore, calling its
destructor is unnecessary.

This is a really wierd thread: I'm the only person who addressed the
orginal poster. There are a bunch of snipers who aren't contributing
anything useful, but keep posting nitpicking nonsense that doesn't
address the real issues here. Instead of posting syllogisms, why don't
you ask the orginal poster why he needs to call the destructor? In any
event, I've given my answer; it works. I have nothing further to say.

 

[BigBrian]

Thank you for your answer. Now, I understand what I need to do.

 

[Andre Kostur]

[stuff snipped in parent posts has been replaced]

[Stuff unnecessarily replaced has been snipped]

The behaviour of dereferencing p_foo is undefined if Foo
is not a POD (because non-PODs can only be correctly
constructed by calling their constructor).

So we should assume that Foo is POD. Therefore, calling its
destructor is unnecessary.

This is a really wierd thread: I'm the only person who addressed the
orginal poster. There are a bunch of snipers who aren't contributing
anything useful, but keep posting nitpicking nonsense that doesn't
address the real issues here. Instead of posting syllogisms, why don't
you ask the orginal poster why he needs to call the destructor? In any
event, I've given my answer; it works. I have nothing further to say.

Certainly not my intent to "snipe", neither do I think my points were
"nitpicking", and the first two points do address the OP. And the first
point is a common error that inexperienced C++ programmers do. My intent
is to inform/remind the OP and other people who may be reading the thread
(and may not have the knowledge that the various experienced people here
do) The points I was trying to make:

1) memory that was new[]ed must be delete[]ed. The OP was new[]ing but
using only delete.
2) newing to one type (char *) and deleting from an unrelated type (Foo
*) is Undefined Behaviour. (No virtual destructor between the types)
3) In your adjusted code you make an explicit call to the destructor of
Foo. Note that there is no mention of a constructor ever being invoked.
So what's the point of the destructor? As Old Wolf pointed out, if this
is a POD, the destructor is unecessary, and if it isn't a POD, then a
contstructor should have been invoked somehow (placement new?).

 

[Pete Becker]

1) memory that was new[]ed must be delete[]ed. The OP was new[]ing but
using only delete.
2) newing to one type (char *) and deleting from an unrelated type (Foo
*) is Undefined Behaviour. (No virtual destructor between the types)
3) In your adjusted code you make an explicit call to the destructor of
Foo. Note that there is no mention of a constructor ever being invoked.
So what's the point of the destructor? As Old Wolf pointed out, if this
is a POD, the destructor is unecessary, and if it isn't a POD, then a
contstructor should have been invoked somehow (placement new?).

I must have missed the message where you pointed all this out to the
original poster and gave him advice on how to solve his problem.

 

[Old Wolf]

This is a really wierd thread: I'm the only person who
addressed the orginal poster.

Your 'advice' either causes undefined behaviour (if Foo is non-POD)
or includes a confusing no-op (if Foo is non-POD).

There are a bunch of snipers who aren't contributing
anything useful

Andre Kostur and I were pointing out the mistake in your advice.
The OP might have been under the impression that your advice
was useful, so we were correcting him of that impression.

Instead of posting syllogisms, why don't you ask the orginal
poster why he needs to call the destructor?

It was you who introduced the idea of calling the destructor.

In any event, I've given my answer; it works.

In this newsgroup, 'works' usually isn't applied to
constructs that cause undefined behaviour.

The OP has, in fact, gotten what he wanted from the thread
(as he mentioned twice), I'm not sure why you think he's
incapable of reading comments not addressed directly to him.

The OP's question was nothing more than "Does my code leak
memory, or does it do the right thing?". A helpful person
could extend this with "Is there a better way to do what I
want?"

To give a more complete answer, as you seem to be requesting
from the other posters in the thread, would require knowing
whether Foo is POD or not. If it is, then the answer to both
questions is:
delete [] p_char;
as has been said.

If Foo is non-POD, then a correct and helpful answer would be
much more complex and would depend on more information from
the OP. If this is the case then he should indicate it. A
helpful solution would have to involve changing the way the
object is constructed (for example, using placement new).

 

[Pete Becker]

Andre Kostur and I were pointing out the mistake in your advice.
The OP might have been under the impression that your advice
was useful, so we were correcting him of that impression.

Gosh, and here I thought that what I suggested was useful, unlike this
pedantic sniping. Did I miss the message where you replied to the OP
with your sage advice on how to solve his problem? Did you try my
suggestion to determine whether it works in practice? No? I didn't think so.

 

[Pete Becker]

Andre Kostur and I were pointing out the mistake in your advice.

No, you were pointing out where you missed the point of my advice.
Unfortunately, you chose to attack rather than ask, and as a result
you're in my killfile once again.