Sign in to follow this  
sriniatig

plz debug this! delete operator error

Recommended Posts

sriniatig    151
Hi I'm writing my own realloc function for my C++ objects. I've written a small prototype function to test it. I'm getting an error where it says "ERROR". Is there any other solution to this ? plz debug this. <code> #include <memory.h> #include <stdio.h> void* redim(void* source, int sourceSize, int targetSize) { fprintf(stderr, "target size: %d\n", targetSize); void* target = new char[targetSize]; fprintf(stderr, "target address: %x\n", target); if(source != NULL) memcpy(target, source, sourceSize); fprintf(stderr, "target address: %x\n", target); fprintf(stderr, "source address: %x\n", source); fprintf(stderr, "ready to crash!\n"); delete source; source = NULL; fprintf(stderr, "AFTER DELETE target address: %x\n", target); return target; } void main() { int *ptr; int *ptr1; // ptr = new (int); ptr = NULL; ptr = (int*)redim(ptr, 0,4); ptr = (int*)redim(ptr, 4,10); ptr = (int*)redim(ptr, 10,10); ptr = (int*)redim(ptr, 10,10); ptr = (int*)redim(ptr, 10,10); ptr = (int*)redim(ptr, 10,10); ptr1 = (int*)redim(ptr, 10,10); ptr1 = (int*)redim(ptr, 10,20); fprintf(stderr, "ERROR!\n"); ptr1 = (int*)redim(ptr, 20,30); ptr1 = (int*)redim(ptr, 30,40); } </code>

Share this post


Link to post
Share on other sites
snk_kid    1312
Quote:
Original post by sriniatig
Hi
I'm writing my own realloc function for my C++ objects.


Then your asking for trouble really, you should never use C memory rountines directly on NON POD-types (Plain old data type), especially realloc it completely messes up C++ objects that are instances of NON POD-types.

Use the c++ standard library vector/deque for dynamic arrays they do automatically grow and do it as efficiently as possible. You can even provide custom allocator types to them.

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by sriniatig
Hi
I'm writing my own realloc function for my C++ objects.


Why?

PS, 'code' tags go in square brackets; they are not HTML.

Share this post


Link to post
Share on other sites
cuBie    122
Hi.

You give NULL to ptr before first redim call, so the delete fail on NULL.

fprintf(stderr, "ready to crash!\n");
delete source;
source = NULL;

change to this:

fprintf(stderr, "ready to crash!\n");
if( source != NULL ) delete source;


This may will work.

Share this post


Link to post
Share on other sites
Helter Skelter    332

BUG:
ptr1 = (int*)redim(ptr, 10,10);
ptr1 = (int*)redim(ptr, 10,20);

fprintf(stderr, "ERROR!\n");
ptr1 = (int*)redim(ptr, 20,30);
ptr1 = (int*)redim(ptr, 30,40);

}



When you call redim() it releases the memory pointed to by 'ptr'. Since the pointer is no longer valid the next time you call redim() it's trying to release and access memory that is no longer available.

You also have a bug in redim(). Setting "source" to NULL only affects the variable in redim(). It has no affect on "ptr" or "ptr1" in main().

You are also mixing data types. Using "new char[50]" to allocate data that is intended to be accessed as an int array will eventually case you problems.

Also, memory allocated as an array should be released using "delete[]" instead of just plain "delete".


You really should go back and rethink your approach. You might want to consider finding someone that is proficient in C++ and has time to act as a mentor.

Share this post


Link to post
Share on other sites
sriniatig    151
we tried this interesting thing with our code. I debugged the code to find out that the pointer is not becoming null in the redim function. So I made the function accept a double pointer and then executed it. It ran perfectly fine!
Here is the code...

<code>
#include <memory.h>
#include <stdio.h>

void* redim(void **source, int sourceSize, int targetSize)
{
fprintf(stderr, "target size: %d\n", targetSize);

void* target = new char[targetSize];

fprintf(stderr, "target address: %x\n", target);

if(*source != NULL)
memcpy(target, *source, sourceSize);

fprintf(stderr, "target address: %x\n", target);
fprintf(stderr, "source address: %x\n", source);

fprintf(stderr, "ready to crash!\n");

delete *source;
*source = NULL;

fprintf(stderr, "AFTER DELETE target address: %x\n", target);
return target;
}

void makeNull(void **ptr)
{
*ptr = NULL;
}

void main()
{
int *ptr;
int *ptr1;

int *ptr2;
ptr2 = (int *)new char[4];
*ptr2 = 10;

makeNull((void **)&ptr2);

ptr = NULL;
ptr = (int*)redim((void **)&ptr, 0,4);

ptr = (int*)redim((void **)&ptr, 4,10);

ptr1 = (int*)redim((void **)&ptr, 10,10);
ptr1 = (int*)redim((void **)&ptr, 10,20);

fprintf(stderr, "ERROR!\n");
ptr1 = (int*)redim((void **)&ptr, 20,30);
ptr1 = (int*)redim((void **)&ptr, 30,40);
}

</code>

Share this post


Link to post
Share on other sites
snk_kid    1312
Quote:
Original post by cuBie
You give NULL to ptr before first redim call, so the delete fail on NULL.

if( source != NULL ) delete source;


This may will work.


[rolleyes] it is permitted to pass null pointers to operators delete/delete[].

Quote:
Original post by sriniatig
It ran perfectly fine!


Now try that code on a dynamic array of a NON POD-type (which typically most C++ user-defined types are) and see what happens.

Also listen to what Helter Skelter told you, your mixing the wrong operators new/delete.

In any case you should be using vector/deque for dynamic arrays in C++.

Share this post


Link to post
Share on other sites
sriniatig    151
delete call will not crash even if the pointer is NULL. and on the delete function calling mistake ( we shoul have used delete [] ptr) will still not solve the problem. We understood the referencing problem and so we used double pointers.

Share this post


Link to post
Share on other sites
snk_kid    1312
Quote:
Original post by sriniatig
delete call will not crash even if the pointer is NULL.


As already mentioned previously.

Quote:
Original post by sriniatig
We understood the referencing problem and so we used double pointers.


Now understand that the code is not only naive its not going to work correctly for certain C++ objects in particular NON POD types.

Share this post


Link to post
Share on other sites
sriniatig    151
I'm currently using it in my program which has custom classes and structures. I ran a test and it is working fine. Plz tell me why will it fail on Non POD types. Uptil now it is working fine. It was a good learning experience uptill now but I'm still not clear on why it should not work on non pod types.
plz advice.

Share this post


Link to post
Share on other sites
sriniatig    151
sorry to assume too soon! i discovered that I'm wrong. It doesnt work on non pod datatypes. will change appraoch. will use vector/deque.

Share this post


Link to post
Share on other sites
Enigma    1410
Well, for a start delete([])ing a void * like you do is undefined behaviour (here I use delete, but the same is true for delete[]):
#include <iostream>

class BasicClass
{

public:
~BasicClass()
{
std::cout << "~BasicClass\n";
}
};

class ClassWithVirtualFunction
{
public:
virtual ~ClassWithVirtualFunction()
{
std::cout << "~ClassWithVirtualFunction\n";
}
};

class DerivedClass
:
public ClassWithVirtualFunction
{
public:
virtual ~DerivedClass()
{
std::cout << "~DerivedClass\n";
}
};

void del(void * obj)
{
delete obj;
}

int main()
{
BasicClass * bc = new BasicClass();
ClassWithVirtualFunction * cwvf = new ClassWithVirtualFunction();
ClassWithVirtualFunction * dc = new DerivedClass();
del(bc);
del(cwvf);
del(dc);
}

None of the destructors are called on any of the three compilers I tested.

Enigma

Share this post


Link to post
Share on other sites
Helter Skelter    332
Quote:
Original post by sriniatig
I'm currently using it in my program which has custom classes and structures. I ran a test and it is working fine. Plz tell me why will it fail on Non POD types. Uptil now it is working fine. It was a good learning experience uptill now but I'm still not clear on why it should not work on non pod types.
plz advice.


Because you are allocating an array of objects rather than an array of pointers (pointing to objects). For primitive types you can get away with it. But for objects the second you allocate new storage, copy the contents, and delete the old storage ALL previous references to those objects become invalid. You are also imposing a restriction on object instantiation. All objects will be instantiated via their default constructors. For some applications this is fine. for others it's not. Some objects are designed that way (and usually include an Init or Create method) - if not you're screwed.

You'll really be in trouble if you start merging arrays that contain different object types even if they share the same inheritence tree. You end up allocating an array based on the assumption that all objects are created equal. If even one is larger than the others the second you copy all the data over you just corrupted one or more objects in the array. This doesn't even take into account the differences in virtual tables from between different compilers. If you start mixing objects and get lucky they will all be the same size. But if one set uses virtual inheritence while others do not the behavior is undefined.

Another bad habit typical of people just learning C++ is the use of typecasing. Typecasting is a religious issue. Everyone has their own views on it. Personally I would be happy to see C style typecasting removed from the C++ spec. Here is my view of typecasting:

Typecasting is BAD. The whole point of using a statically typed language is to ensure that you don't mistake one object type for another. Anytime you use typecasting you need to take a step back and ask yourself why you are doing it. The majority of typecasts are the result of bad design and lack of experience. If you must absolutely use typecasts turn on RTTI and use the dynamic_cast operator - even if it's ONLY in debug mode.




While you might think it's a great learning experience toying around with this type of implementation I can assure you is it not. The more you do it the more it becomes habit. Since you are obviously not familiar with the language this equates to VERY BAD style since you are getting by without realizing the implications and serious bugs are being missed.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this