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[i];
}

Thanks Everybody in advance.

[Phlip]

Nemo wrote:

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[i];
}

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!

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!

[Jay_Nabonne]

On Thu, 23 Feb 2006 15:26:35 -0500, Nemo wrote:

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.]

In article <dt**********@dns3.cae.ca>, "Nemo" <nemo@no_email.com>
wrote:

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[i];
}

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.
--
Magic depends on tradition and belief. It does not welcome observation,
nor does it profit by experiment. On the other hand, science is based
on experience; it is open to correction by observation and experiment.

[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

"Phlip" <ph*******@gmail.com> wrote in message
news:D%*****************@newssvr24.news.prodigy.ne t...

Nemo wrote:

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[i];
}

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!

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!

[Nemo]

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

"Jay_Nabonne" <jn******@pacbello.net> wrote in message
news:pa****************************@pacbello.net.. .

On Thu, 23 Feb 2006 15:26:35 -0500, Nemo wrote:

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

"Daniel T." <po********@earthlink.net> wrote in message
news:po******************************@news.east.ea rthlink.net...

In article <dt**********@dns3.cae.ca>, "Nemo" <nemo@no_email.com>
wrote:

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[i];
}

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.
--
Magic depends on tradition and belief. It does not welcome observation,
nor does it profit by experiment. On the other hand, science is based
on experience; it is open to correction by observation and experiment.

[Ian Collins]

Nemo wrote:

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....

--
Ian Collins.

[Phlip]

Ian Collins wrote:

Nemo wrote:

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.

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.

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!

[Clark S. Cox III]

On 2006-02-23 16:09:50 -0500, "Nemo" <nemo@no_email.com> said:

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
--
Clark S. Cox, III
cl*******@gmail.com

[Andrew Koenig]

"Nemo" <nemo@no_email.com> wrote in message news:dt**********@dns3.cae.ca...

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]

Nemo wrote:

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?

--
Ian Collins.

[Daniel T.]

In article <dt**********@dns3.cae.ca>, "Nemo" <nemo@no_email.com>
wrote:

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?
--
Magic depends on tradition and belief. It does not welcome observation,
nor does it profit by experiment. On the other hand, science is based
on experience; it is open to correction by observation and experiment.

[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
"Nemo" <nemo@no_email.com> wrote in message news:dt**********@dns3.cae.ca...

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[i];
}

Thanks Everybody in advance.