why delete operator crashes

[Nemo]

Hello Folks,
I need to manipulate a list of char strings as follows, but when I want to
delete the pointer created with new at the end, delete operator crashes, any
idea?

char* list[2000];
while(...)
{
list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';
MyCounter++;
}

for (i=0; i<MyCounter;i++)
{
delete [] list;
}

Thanks Everybody in advance.

 

[Phlip]

char* list[2000];

Add this line:

assert(NULL == list[0]);

It might fail.

(It also might pass, so don't rely only on it.)

for (i=0; i<MyCounter;i++)
{
delete [] list;
}

You are allowed to delete[] a NULL pointer. But you should not delete a
garbage pointer; one containing an undefined value not returned by new.

All uninitialized stack variables and heap variables contain garbage. Always
initialize, at least like this:

char *list[2000] = { NULL };

Now learn to use std::string and stop writing high-risk code with raw
pointers!

 

[Jay_Nabonne]

char* list[2000];
while(...)
{
list[MyCounter] = new char [Stringlength];

Should be:

list[MyCounter] = new char [Stringlength+1];

strncpy(list[MyCounter], c_TempFilename, Stringlength);

....since you're sticking a 0 on the end.

list[MyCounter][Stringlength] = '\0';
MyCounter++;
}

- Jay

 

[Daniel T.]

Hello Folks,
I need to manipulate a list of char strings as follows, but when I want to
delete the pointer created with new at the end, delete operator crashes, any
idea?

char* list[2000];
while(...)
{
list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';

Off by one error, list[MyCounter] has no element at [Stringlength].

MyCounter++;
}

for (i=0; i<MyCounter;i++)
{
delete [] list;
}

Thanks Everybody in advance.

The above problem is probably clobbering whatever code your delete
operator needs in order to delete the array.

I strongly suggest you use std::string.

std::vector<std::string> list;
while ( ... )
{
list.push_back( std::string( c_TempFilename ) );
}

// no delete needed.

 

[Nemo]

Phlip,
I wish I could use std::string, but the code that I am trying to fix is a C
code and compiler does not compile it.
the pointers are not garbage, they are initialized with strings:

list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';

Regartds

char* list[2000];

Add this line:

assert(NULL == list[0]);

It might fail.

(It also might pass, so don't rely only on it.)

for (i=0; i<MyCounter;i++)
{
delete [] list;
}

You are allowed to delete[] a NULL pointer. But you should not delete a
garbage pointer; one containing an undefined value not returned by new.

All uninitialized stack variables and heap variables contain garbage. Always
initialize, at least like this:

char *list[2000] = { NULL };

Now learn to use std::string and stop writing high-risk code with raw
pointers!

 

[Nemo]

Jay,
that was the problem. Thanks. Now it works.
Regards

char* list[2000];
while(...)
{
list[MyCounter] = new char [Stringlength];

Should be:

list[MyCounter] = new char [Stringlength+1];

strncpy(list[MyCounter], c_TempFilename, Stringlength);

...since you're sticking a 0 on the end.

list[MyCounter][Stringlength] = '\0';
MyCounter++;
}

- Jay

 

[Nemo]

Daniel,
As I mentioned before, I would love to use templates but in this code
compiler does not allow me to use them. So I am stuck with their old style.
thanks.
Regards

Hello Folks,
I need to manipulate a list of char strings as follows, but when I want to
delete the pointer created with new at the end, delete operator crashes, any
idea?

char* list[2000];
while(...)
{
list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';

Off by one error, list[MyCounter] has no element at [Stringlength].

MyCounter++;
}

for (i=0; i<MyCounter;i++)
{
delete [] list;
}

Thanks Everybody in advance.

The above problem is probably clobbering whatever code your delete
operator needs in order to delete the array.

I strongly suggest you use std::string.

std::vector<std::string> list;
while ( ... )
{
list.push_back( std::string( c_TempFilename ) );
}

// no delete needed.

 

[Ian Collins]

Phlip,
I wish I could use std::string, but the code that I am trying to fix is a C
code and compiler does not compile it.

But it compiles new and delete....

 

[Phlip]

I should have caught the issue Daniel T. did. I stopped at the first thing I
didn't see - uninitialized pointers.

You should implement my fix as well. And in general, you should C++ this C
code as much as possible.

But it compiles new and delete....

I can imagine a legitimate C++ compiler compiling legacy C code and getting
stuck on std::string.

(I can also imagine a poster just a little naive to C++, and forgetting to
#include <string>.)

I can't imagine the compiler handling new and delete but not templates. The
suggestions to Nemo are...

- ensure you study C++ independently, in its native environment
- write unit tests
- after this code works, refactor it to extract _little_ functions
- make sure the compiler itself is robust and not causing trouble
- carefully examine all the code for undefined behavior
- fix any excesses, possibly re-using the little extracted functions

Migrating to pure C++ is only possible after this C/C++ hybrid is
_completely_ under control and safe to change.

 

[Clark S. Cox III]

Phlip,
I wish I could use std::string, but the code that I am trying to fix is a C
code and compiler does not compile it.

If you're writing C code, then you can't use new or delete either.

the pointers are not garbage, they are initialized with strings:

list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';

bzzzzt. There's your error, you've run off the end of the array. What
you've done is essentially the same as:

char foo[5];
foo[5] = '\0';
//foo has 5 elements: [0],[1],[2],[3],[4]
//foo[5] is *not* a valid part of foo

 

[Andrew Koenig]

the pointers are not garbage, they are initialized with strings:

list[MyCounter] = new char [Stringlength];
strncpy(list[MyCounter], c_TempFilename, Stringlength);
list[MyCounter][Stringlength] = '\0';

This code is wrong. If you use "new char[Stringlength]" to initialize a
variable, then the elements of that variable have indices of 0 through
Stringlength-1, inclusive. Therefore, when you execute

list[MyCounter][Stringlength] = '\0';

you are overwriting memory outside the bounds of the array that you
allocated, and the effect is undefined.

 

[Ian Collins]

Daniel,
As I mentioned before, I would love to use templates but in this code
compiler does not allow me to use them. So I am stuck with their old style.

Please don't top post.

I'm curious, which compiler is this, Embedded C++ maybe?

 

[Daniel T.]

Daniel,
As I mentioned before, I would love to use templates but in this code
compiler does not allow me to use them. So I am stuck with their old style.
thanks.

Then implement a vector and string class yourself, without using
templates. It's not that hard... Just a thought, are you using C++ or C?
Maybe you are in the wrong group?

 

[Nemo]

Guys,
Sorry, you are all right, my mistake. I was testing my function with a C++
compiler and I did not notice that the C compiler that compiles my main code
does not compile new and delete. But you were right about the problem in my
code, I was trying to access invalide place because of wrong size. Thanks
every body, I learned something.
Regards
Nemo