tags:

views:

109

answers:

3

Hi

I am working on some strange piece of code ,For me its not good piece of code.

PIP_ADAPTER_INFO pAdapterInfo=(PIP_ADAPTER_INFO)new  
                               char[sizeof(IP_IP_ADAPTER_INFO)];

.
.
.
delete []pAdapterInfo;

Here PIP_ADAPTER_INFO is pointer to struct IP_IP_ADAPTER_INFO , size of IP_IP_ADAPTER_INFO is 640.

I was expecting crash in delete []pAdapterInfo call.But there is no crash.I wrote an small test code.

        class TestClass
        {

        public:
        /*  TestClass()
            {
            }
            ~TestClass()
            {
            }*/

        public:
            int array[10];
        };

        int main (int ac, char **av)

        {
            TestClass *myptr=(TestClass*) new char[10];
            delete []myptr;
            return 0;
        }

What i see :

  1. If i un-comment the c'tor and d'tor, test code crashes (assert fails)
  2. If i keep it commented nothing fails.

Even if i see disassemble , it is different in both case above

/*****************************************************************/
/********Compiler provided c'tor and d'tor ***********************/
/*****************************************************************/
28:       TestClass *myptr=(TestClass*) new char[10];
00401268   push        0Ah
0040126A   call        operator new (004082d0)
0040126F   add         esp,4
00401272   mov         dword ptr [ebp-8],eax
00401275   mov         eax,dword ptr [ebp-8]
00401278   mov         dword ptr [ebp-4],eax
29:       delete []myptr;
0040127B   mov         ecx,dword ptr [ebp-4]
0040127E   mov         dword ptr [ebp-0Ch],ecx
00401281   mov         edx,dword ptr [ebp-0Ch]
00401284   push        edx
00401285   call        operator delete (004060d0)
0040128A   add         esp,4
30:
/*****************************************************************/
/********User provided c'tor and d'tor ***********************/
/*****************************************************************/
28:       TestClass *myptr=(TestClass*) new char[10];

28:       TestClass *myptr=(TestClass*) new char[10];
00401278   push        0Ah
0040127A   call        operator new (004083e0)
0040127F   add         esp,4
00401282   mov         dword ptr [ebp-8],eax
00401285   mov         eax,dword ptr [ebp-8]
00401288   mov         dword ptr [ebp-4],eax
29:       delete []myptr;
0040128B   mov         ecx,dword ptr [ebp-4]
0040128E   mov         dword ptr [ebp-10h],ecx
00401291   mov         edx,dword ptr [ebp-10h]
00401294   mov         dword ptr [ebp-0Ch],edx
00401297   cmp         dword ptr [ebp-0Ch],0
0040129B   je          main+4Ch (004012ac)
0040129D   push        3
0040129F   mov         ecx,dword ptr [ebp-0Ch]
004012A2   call        @ILT+0(TestClass::`vector deleting destructor') (00401005)
004012A7   mov         dword ptr [ebp-14h],eax
004012AA   jmp         main+53h (004012b3)
004012AC   mov         dword ptr [ebp-14h],0

Please help me with your expertise to learn this feature of C++ .

Thanks in advance.

Sat

+4  A: 

$5.3.5/3 - "In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.73)"

So, what you are seeing is infact undefined behavior.

73) This implies that an object cannot be deleted using a pointer of type void* because there are no objects of type void.

Chubsdad
I made the necessary corrections to the quote(answer). Hope you don't mind. :)
Prasoon Saurav
@Prasoon Saurav: :)
Chubsdad
+12  A: 

I'm assuming here that by IP_IP_ADAPTER_INFO you mean Windows' IP_ADAPTER_INFO structure. Even if not, the gist of this is the same: your code is leading to undefined behaviour, and it's the fault of whoever wrote it. Fix it immediately.

You allocated an array of char with new, but then free that memory as if it were an array of IP_ADAPTER_INFO. C++ doesn't know that you're lying to it, so it goes and tries to treat your char array as an IP_ADAPTER_INFO array, and then dies horribly when it finds out the awful truth.

Now, this works sometimes because VC++ records enough info about the allocated memory that delete[] doesn't care about the pointer's type, but this is evil evil wrong bad illegal get-you-taken-out-back-and-shot code.

It may work on your particular compiler, but that's entirely a fluke. You should instead be doing:

PIP_ADAPTER_INFO pAdapterInfo = new IP_ADAPTER_INFO;
//DoSomethingToAdapterInfo(pAdapterInfo);
delete pAdapterInfo;

But even then, unless you need to hold onto that structure in a global scope, which itself points to bad design, you really shouldn't be using new and delete here at all. You should be doing something closer to this:

IP_ADAPTER_INFO adapterInfo;
//DoSomethingToAdapterInfo(&adapterInfo);

Letting C++ handle allocation and deletion (on the stack) for you. If you need to return the structure, return it rather than a pointer to it (so your caller doesn't need to worry about memory management.)

If there's some obscure or unique reason why you are using heap allocation instead of stack allocation, then you may be justified in doing so--but even then, casting new char[...] to PIP_ADAPTER_INFO is bad.

Jonathan Grynspan
For a detailed answer, and for "C++ doesn't know that you're lying to it", +1
JoshD
I agree that from a design standpoint returning the structure is better than returning a pointer to a heap object, but I would like to point out that for performance reasons the latter might become necessary. (But never forget premature optimization is the root of all evil...)
Philipp
I am always wary of copy constructors when programming in C++. I loathe the very idea of them--"surely," says my brain, "compilers could be smart enough to, upon seeing the return value of a function being assigned directly to a variable in the caller on first use, so that something like `MyStruct foo = GetAStruct();` need never use the copy constructor," but then when I absolutely can't have the overhead, I return a smart pointer rather than a raw pointer and all is well.
Jonathan Grynspan
(Of course, the overhead incurred by smart pointer usage is non-nil, but in situations where the copy constructor is so expensive as to warrant optimization, the smart pointer's gonna be peanuts.)
Jonathan Grynspan
(Copy constructors and assignment operators going hand in hand, of course.)
Jonathan Grynspan
A: 

the original is a bad design, and undefined for multiple reasons.

delete []pAdapterInfo; should read: delete pAdapterInfo; if you're working with a single object.

considering the statement: char[sizeof(IP_IP_ADAPTER_INFO)] you're treating it as one object, and should be using new/delete.

your q(+a) lies in understanding of new/delete implementation vs. new[]/delete[] on any/all platforms.

an implementation of new[]/delete[] could place its own private header data in the allocation to note the number of objects allocated in order to destruct an array properly. how new[]/delete[] differs and manages its allocations is all implementation defined. a single allocation using new/delete typically omits this data (again, implementation defined).

undefined behaviour is also applicable because the alignment is allowed to differ.

Justin
Actually, `delete pAdapterInfo` is just as wrong. IF anything, it should be `delete[] reinterpret_cast<char*>(pAdapterInfo)`.
sbi
@sbi that.doesn't.matter. it's still undefined. please, read beyond 1.5 sentences before making such comments - it appears that you missed a few points i was making :-\
Justin
Agreeing with sbi here; the first few sentences just don't make sense. Nor does the last, actually: `new char[N]` must be aligned properly to hold any object with `sizeof(object) <= N`.
MSalters
@MSalters alignment is implementation defined. new allocation requests account for the objects' natural alignment. what in the standard leads you to believe (and ensures) that the natural alignment of `char[]' >= the natural alignment of an arbitrary struct?
Justin
@Justin: 3.7.3.1/2
MSalters