Weird memory-associated error

[tA-Kane]

I seem to be having trouble with one of the most fundamental operators of C++: the new and delete operators. Hmmm...

Code Select Expand

int i;
char *saved;
bool success;

// some initialization and sanity checks here
...

// msg->line is a char array
i = strlen(msg->line);
if (i <= 0)
saved = NULL;
else
{
saved = new char[i];
if (saved)
strcpy(saved, msg->line);
}

// middle code that modifies msg->line
...

// now check to see if i want to log the msg, if i do, then restore msg->line and send to logging function
if (!success)
{
if (!saved)
irc_dbg(__FILE__, __LINE__, "failure saving message, hopefully no changes were made");
else
strcpy(msg->line, saved);
irc_log(irc, msg);
if (saved)
delete[] saved;
}
Now, this code looks fine to me, but when I execute it, I get an error...

Debug Error!

Program C:\Program Files\SphtBotv3\SphtBotv3.exe

DAMAGE: after Normal block (#63) at 0x020D1C40.


(Press Retry to debug the application)

I've traced it down to the delete[] call, but I don't think I'm understanding why that's failing...?

I've also tried removing the code between the allocation and the deallocation, thinking maybe I fucked up somewhere and was modifying saved, but no... even with no middle code, it still fails.

Could someone point me in the right direction, please?

[tA-Kane]

Aha, I've figured it out. I was forgetting to include room for the null terminator that strcpy() would create...  :\

Here's the corrected line, for anyone interested:

Code Select Expand

saved = new char[i+1];

[EpicOfTimeWasted]

Another thing is that in your case, new will never return zero, it will throw bad_alloc.  So the if(saved) after the new isn't needed.

[Eibro]

Another thing is that delete and delete[] will take no action on a null pointer, so if ( saved ) delete[] saved; is not necessary.

[Mephisto]

Another thing is that delete and delete[] will take no action on a null pointer, so if ( saved ) delete[] saved; is not necessary.

Why isn't it necessary?  He's checking to make sure that it's necessary to free the allocated memory that's remaining.

Also, isn't it common that delete will crash when operating on a pointer which points to nothing?  I've forgotten under which instances a call to free() or delete will cause the program execution to crash due to operating on an "invalid pointer."

[K]

Another thing is that delete and delete[] will take no action on a null pointer, so if ( saved ) delete[] saved; is not necessary.

Although deleting a NULL pointer is guaranteed by the C++ standard to have no action, quite a few implementations of delete will zonk out if you try to delete a NULL pointer.

[tA-Kane]

Thx for the heads up people. Now I have to fix more. 

As for checking if the pointer is NULL before attempting to free, I'd rather do that locally and be assured that delete isn't going to get screwy with me.

in your case, new will never return zero, it will throw bad_alloc.

When will it return zero?

[EpicOfTimeWasted]

in your case, new will never return zero, it will throw bad_alloc.

When will it return zero?

You can tell new to not throw an exception and instead return zero.

Code Select Expand

saved = new(nothrow) char[i + 1];
if(saved)
  strcpy(saved, msg->line);

[Mephisto]

Thx for the heads up people. Now I have to fix more. 

As for checking if the pointer is NULL before attempting to free, I'd rather do that locally and be assured that delete isn't going to get screwy with me.

in your case, new will never return zero, it will throw bad_alloc.

When will it return zero?

Keep in mind that your check could evaluate to true if a pointer isn't NULL but has not been dynamically allocated and thus would result in a crash if you attempted to delete it.  But I doubt that would happen in any event.

[Eibro]

Thx for the heads up people. Now I have to fix more. 

As for checking if the pointer is NULL before attempting to free, I'd rather do that locally and be assured that delete isn't going to get screwy with me.

in your case, new will never return zero, it will throw bad_alloc.

When will it return zero?

It won't. It's guaranteed to do nothing with a NULL pointer.

[tA-Kane]

I found an interesting article related to a bug in the version of VS I am using (VS6):

http://msdn.microsoft.com/msdnmag/issues/03/09/LegacySTLFix/default.aspx


Hmmm... maybe I should ... "find" ... VS.NET, and use that...  :\

[Kp]

Or switch to MinGW's g++.