On Tue, 7 Nov 2006, Paul Eggert wrote: > "Joel E. Denny" <[EMAIL PROTECTED]> 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.
