Re: simple valgrind test found what appear to be serious memory problems

[Thien-Thi Nguyen]

() Paul Eggert <address@hidden>
() Thu, 30 Sep 2010 09:21:35 -0700

   By "things look as they should" I assume you mean that at the
   affected line (b-grok.c:794, "if (ny->branches)"), things look
   OK, because ny->branches is NULL.  But that doesn't mean there's
   not a problem here.  Although ny->branches happens to be NULL,
   which is what we want, it is indeed uninitialized, and
   it might not be NULL on some other platform.

   You can easily very that ny->branches is uninitialized by
   temporarily trying the following change.

   [b-divv.c ‘alloc’ modification]

Yes, you are right.  Thanks for showing me the difference between "right
by accident" and "right by design".  What we want is the latter but what
we have (sometimes) is the former.

   First, valgrind was working just fine here.  Although valgrind
   is not perfect, it's pretty good, and its diagnostics should not
   be dismissed without understanding exactly why they are false
   alarms.

   Second, there are real advantages to the standard malloc+free forms:
   among other things, programmers (and other tools) will understand
   them better.  Is there a compelling reason for switching to the
   unconventional form?

Generally, there were many small strings (revno fragments and such)
being malloc'd and free'd very soon afterwards, primarily by the parser.
Obstacks seemed to be a better fit for this allocation pattern.

At least, that's how i remember it.  Undoubtedly, there is also
performance, readability and other perceived improvements arising from
the change, not to mention this programmer's ego and curiosity muddying
the waters...

   Third, I'm not happy with the idea of debugging this stuff via
   valgrind.  Valgrind is not meant to be a first line of defense
   against problems in software design or maintenance, and if it
   is finding several problems like this, that's an indication that
   the software development practices need to be changed, perhaps
   by a careful code audit.

I agree completely.  How does one go about such an audit?