Re: simple valgrind test found what appear to be serious memory problems
Thien-Thi Nguyen <[email protected]> Thu, 30 Sep 2010 21:09:35 +0200
| Newsgroups | gmane.comp.version-control.rcs.bugs |
|---|---|
| Message-ID | <[email protected]> |
() Paul Eggert <[email protected]> () 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?