Very Computer

Hello fellow linux developers. I seem to be having some serious problems
with the dynamic memory allocation routines in C++. I am using a pretty
vanilla Slackware 2.0.0- libc 4.5.26, gcc 2.5.8, kernel 1.1.54/62.

What I am trying to do is create a dynamically sized list of char
pointers. When adding one, the old char ** array is reallocated and
the old contents are copied into it. The routine that accomplishes
this is add_stringlist(), below. Another routine involved is clear(),
also below, which deallocates the list once and for all. However,
when the "delete" statement is called, the program crashes with a
segmentation fault. Not on the first time, mind you- it takes a few tries
before it happens- but generally less than ten. I've tried both
"delete" and "delete []." (The function newdup does a strdup but
allocates the memory with new.)

I am completely stumped. Would anyone have any help for me? This was
happening yesterday with another deallocation of a char ** that I was
trying to do as well- I eventually just stopped deallocating it due to
frustration.

A trace from GDB follows, as well as the two code fragments. I appreciate
all responses.

Cheers,

Doug

A trace from GDB 4.12 reveals:

(gdb) where
#0  0x14521 in _free_internal (ptr=0x2000) at free.c:169
#1  0x14602 in free (ptr=0x2000) at free.c:217
#2  0xfe30 in __builtin_delete (ptr=0x2000)

    toadd=0x2c300 "somestring") at somefile.C:103
...
(gdb) list
98        int i;
99      
100       newlist = new char *[count + 1];
101       for (i = 0; i < count; i++)
102         newlist[i] = list[i];
103       delete [] list;
104       list = newlist;
105       list[count++] = newdup(toadd);
106     }
107    

void add_stringlist(int &count, char ** &list, const char *toadd)
{
  char **newlist;
  int i;

  newlist = new char *[count + 1];
  for (i = 0; i < count; i++)
    newlist[i] = list[i];
  delete [] list;
  list = newlist;
  list[count++] = newdup(toadd);

Quote:}

void some_class::clear(void)
// Frees all allocated memory
{
  int i;

# define CINN(x) do { if ((x)) { delete (x); (x) = NULL; } } while (0)
  // clear if not null

  for (i = 0; i < msgcount; i++)
    CINN(messages[i]);
  delete [] messages; // This is causing core dump???
  msgcount = 0;
  ...

# undef CINN

Quote:}

Quote:>...when the "delete" statement is called, the program crashes with a
>segmentation fault. Not on the first time, mind you- it takes a few tries
>before it happens- but generally less than ten. I've tried both
>"delete" and "delete []."

Mixing and matching is not allowed.
If you new, then delete.
If you new[], then delete[].
If you malloc(), then free().

Any other permutation is ill-formed.

If you new and new[] to the same pointer, then the program needs
to keep track of it if it is going to delete.

Quote:>(The function newdup does a strdup but allocates the memory with new.)

Eh?

Quote:>I am completely stumped. Would anyone have any help for me? This was
>happening yesterday with another deallocation of a char ** that I was
>trying to do as well- I eventually just stopped deallocating it due to
>frustration.

>A trace from GDB follows, as well as the two code fragments. I appreciate
>all responses.

So long as pointers, and other C and C++ things are around,
perhaps some philosophy might be applicable here.
There is an old saying which goes as follows:

What you see is what you get.
-- Anonymous

That doesn't always fly.
A well noted chap has been quoted as outsmarting it with:

What you see is all you get.
-- Brian Kernighan

IMO, that doesn't always fly either.  Another well noted chap
has been know to say:

What you see is what you see.  What you get is what you get.
-- Greg Comeau :-)

Ok, what's my point? De* output can be misleading.
That you say it goes a few times around and such shows that perhaps
something else is actually the culprit rather than the line a
de* may be showing.

Quote:>void add_stringlist(int &count, char ** &list, const char *toadd)
>{
>  char **newlist;
>  int i;

>  newlist = new char *[count + 1];
>  for (i = 0; i < count; i++)
>    newlist[i] = list[i];
>  delete [] list;
>  list = newlist;
>  list[count++] = newdup(toadd);
>}

You assign newlist to list, and apparently the next time through
new an array, so delete'ing an array (the previous one) is the right thing,
assuming you don't reassign any of this anywhere else.
Have no idea what a newdup() is, but you try to say something about
it above.

BTW, an aside: this can turn out to be an expensive situation,
just adding space for one slot at a time that is.  Of course,
every app doesn't need grease, so you may not care.

Quote:>void some_class::clear(void)
>// Frees all allocated memory
>{
>  int i;

># define CINN(x) do { if ((x)) { delete (x); (x) = NULL; } } while (0)
>  // clear if not null

>  for (i = 0; i < msgcount; i++)
>    CINN(messages[i]);
>  delete [] messages; // This is causing core dump???
>  msgcount = 0;
>  ...

># undef CINN
>}

Above you imply this gets called to clear the list.
If that is the case, then you have to clarify newdup
as your code is inconsistent and hence non-revealing.
That is, in clear you delete the elements of the array.
You do not do so in add_stringlist.  I am not saying to
delete them, because I cannot see all your code, so instead
that you should figure out which is which and get them all the same.
Further, this may very well be related to your problem.
Ditto for that your count identifiers might be off by one,
either here, or when the program starts, etc.

Another aside: there are a number of considerations I'll offer
for food for thought wrt CINN:
o Macro's should be used sparingly.  I cannot see its purpose here.
o delete check's to see if if has a `null pointer' before doing
  anything, so you don't need to check again.
  (Yes, I see the assignment to clear the pointer.)
o I do not want to open a war here, but rather would refer you
  to texts that discuss the use of NULL in C++.
o I don't understand why you're using the do while here.
o Apparently the point of your using CINN here is so that you do
  not have to keep typing messages[i].  I am not clear how noble
  that is, especially given my comments above, and also that
  you delete [] messages in the next line anyway, so making the
  elements null pointers didn't help the program.
o I do recognize as well that the bug may have evolved your
  program into using CINN so that you could investigate the problem
  more rapidly, or somesuch.  In that case, these comments can still
  be helpful to others seeing this then.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

Quote:Greg Comeau writes:
> If you new, then delete.
> If you new[], then delete[].

They should warn about that in GCC.  Your post just saved me
probably days of bug-seeking.  Thanks.

ralf
--
You just began to read the sig and you have finished it now.

Quote:>Greg Comeau writes:
>> If you new, then delete.
>> If you new[], then delete[].

>They should warn about that in GCC.  Your post just saved me
>probably days of bug-seeking.  Thanks.

Oddly enough: maybe.  Not that C++ compilers are unable to
detect any of such situations, but in the general case, it
cannot, or is harder, and I am not aware of any that do.
For instance, consider:

SomeType *p;
...
while (somethingorother) {
    ...
    p = new SomeType;
    ...
    if (blah)
        break;
    ...
    p = new SomeType[5];
    ...

Quote:}

...
delete ???;

Which form should it be?  Only your program's logic knows.  Yes?

If you want to "force" the issue, then new SomeType[1] on the former case,
but that's somewhat another issue.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

That's not 100% true, and not the reason for the different syntax's.
Just delete could suffice, however that implies space and time
tradeoffs because then new would have to record whether or not an
array was allocated.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

Please, oh please, read what you wrote and what I wrote.
And please, oh please, go look at something like the ARM.
Relax.  I know what you said.  In fact, I posted said in at least
one other message (including an example similar to yours),
however, it is still not the reason for the different syntaxes.
That is can or cannot be detected at compile time _is not_
the [full] issue.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

It is neither detectable at compile *nor* at run time.

As Jerry Schwarz pointed out very friendly (when I brought up this
topic back in 1991 on comp.std.c++) the problem at hand is user
specified class memory management. Let's see, I still have it somewhere...

----------------
Hmm.  If I can say

        T* p1 = new T[10] ;
        T* p2 = new T ;
        delete p1 ;
        delete p2 ;

Then the system must be stuffing the fact that p2 was allocated
as a single T somewhere.  The space for saving this fact comes
from somewhere. If it comes from T::new you will be changing what
T::new is expected to do.  If it comes from ::new, then you are
violating the principal that T::operator new is in complete
control of allocation of T's.  Note that some people use
local operator news for threads and/or shared memory stuff.
----------------

        Joachim

--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

Computer Science Department
Technical University of Darmstadt, Germany

Sigh.  There need not be runtime checking!

We agree on (a) (at least that in enough cases it cannot detect it,
if you mean all cases, then we disagree :-{).

(b) does not follow because a tradeoff would still exist:
it did not want to be left as an implementation detail, it's that simple.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

Yes, and "the space" can take on many forms, hence complicating
the problem.

Quote:> If it comes from T::new you will be changing what
>T::new is expected to do.  If it comes from ::new, then you are
>violating the principal that T::operator new is in complete
>control of allocation of T's.  Note that some people use
>local operator news for threads and/or shared memory stuff.

This is barking up the right tree.

- Greg
--
       Comeau Computing, 91-34 120th Street, Richmond Hill, NY, 11418-3214

    Voice:718-945-0009 / Fax:718-441-2310 / Prodigy: tshp50a / WELL: comeau

: >It is neither detectable at compile *nor* at run time.

Of course its detectable at runtime. Every pointer just needs
a tag which gets set during new. Then delete merely checks the
tag. Copying pointers should also copy the tag (duh!).

I dont see how you reliably detect the difference during compile
time. I dont think you can! (MHO only). You said this though.

Who stores and manages this tag?

        ::new or T::new ?

We're talking about overloaded operators here, don't forget this.
Note also that T::new must be in full control over memory
management, by definition.

Obviously you stopped reading my article after the sentence you
quoted above. Please read the problem description again.

        Joachim

--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

Computer Science Department
Technical University of Darmstadt, Germany

I was under the impression that the programmer has no official information
on what a pointer looks like, how big it is, or anything except its behavior
under a few operators.  Therefore, I'd think ::new is allowed to do just
about anything.
                                        Steve