Re: Valgrind and Bison memory leaks

[Joel E. Denny]

On Tue, 7 Nov 2006, Paul Eggert wrote:

> "Joel E. Denny" <address@hidden> writes:
> 
> > I try to use Valgrind to make sure I don't create new memory leaks in 
> > Bison.  However, Valgrind's complaints about all the existing memory leaks 
> > really get in the way.
> 
> My kneejerk reaction is that these changes make the code harder to
> follow,

There's an effort throughout Bison to clean up the heap when exiting with 
status 0.  As far as I can tell, the memory leaks I've found are arbitrary 
exceptions and thus make the code harder to follow.  Before pursuing this 
issue with Valgrind, I had already stumbled through some of this code 
feeling confused about if and where memory is actually freed.  I'm hoping 
my changes will clear up all that confusion for other developers as well 
as for myself.

> harder to maintain

Assuming my changes do not make the code harder to follow, I don't see 
that there's any significant increase in maintenance difficulty.

> , bigger,

By very little, I believe.

> and slower.

Significantly?

> (I was particularly
> bemused by the union of const char * and char *.  :-)

The memory management in muscle_tab.c is already confusing to me.  Trying 
to figure it out was actually the very issue that prompted me to start 
testing Bison rigorously with Valgrind.

I found that, in order to avoid illegal free's and leaked memory, Bison 
consistently avoids mixing muscle_insert and muscle_*grow on the same 
entry's value.  The interface is such that muscle_insert leaves 
responsibility for the value's memory with the caller, but muscle_*grow 
duplicates the specified value and assumes responsibility only for its 
copy.  That is, muscle_insert internally maintains a value as if it were a 
const char * while muscle_*grow maintains it as a char *.

I just want to make this functionality explicit and enforce it with aver 
and const so that no one will have trouble working with it.  (More code 
comments would probably help too.)  Then I can confidently invoke 
code_scanner_free, which has been hanging around for a while unused.  
Also, I can store a const char * in muscle_table without gcc complaining.

Alternatively, muscle_insert could duplicate the input value and free the 
old value just like muscle_*grow.  That would be fine with me.  However, I 
figured that wasn't being done for efficiency reasons.