On Sat, May 16, 2020 at 10:05 AM Bruno Haible <br...@clisp.org> wrote: > > Hi all, > > Let me try to bring some structure into this discussion. > > 1) The memory-leak tools > 2) The developer's perspective > 3) The QA automation perspective > > > 1) The memory-leak tools > ======================== > > Paul and I apparently use the tools differently [1]. So, I've written > a wiki page > https://gitlab.com/ghwiki/gnow-how/-/wikis/Finding_memory_leaks > that describes what memory leaks are, what tools exist to find them, > what are advantages and drawbacks. Please read it. > > (If you want to contribute to this wiki, drop me a note, and I'll give > you write access.) > > In particular, all three tools by default omit memory leaks caused by > global and static variables. > > Paul writes in [1]: > > In any event, it seems to me to be a deficiency in the detection if it > > reports allocated memory which is still referenced to by global > > variables, or even static variables, as memory leaks. > > On the contrary, reporting leaks through global and static variables is > a feature. *Hiding* them, which is what the tools do by default, is a > *misfeature*. > > Why? Because caches that grow without bounds are serious memory leaks > as well. It is pointless to eliminate serious memory leaks that were > caused by forgetting free(), while at the same not noticing serious memory > leaks that were caused by bad design of caches. BOTH have same adverse > effects. > > This means, when looking for memory leaks, you should use valgrind with > '--show-reachable=yes'. > > > 2) The developer's perspective > ============================== > > A developer will want to eliminate all serious memory leaks and not spend > much time on the not-serious ones. However: > > The tools cannot distinguish serious from not serious memory leaks. > > Therefore the developer will typically want to silence the not serious > memory leaks (like they also want to silence "gcc -Wall" warnings when > they are pointless). > > I can see three good and one bad ways of doing so: > > * [good] Enhance the suppressions file(s). > > * [good] In unit tests, add free() statements right before exit(). Like > Paul said in [2]: > "tests should not contain those types of memory leaks and if someone > comes along with a fix, it should be applied." > > * [acceptable] In production binaries, add free() statements guarded by an > environment variable test: > > if (getenv ("CLEANUP_BEFORE_EXIT") != NULL) > free (data); > exit (0); > > Removing the getenv test would, however, violate the GNU standards [3]. > > * [bad] Omit the valgrind option '--show-reachable=yes'. > This is bad because, as explained above, it will make you blind against > some types of serious memory leaks. > > > 3) The QA automation perspective > ================================ > > For QA automation, a multitude of program executions are done with a > memory leak checker enabled. Since it needs to be automated, the usual > policy will be that a valgrind or leak sanitizer report is considered > a failure. > > The QA automation needs to rely on the suppression files created by the > developers, since it's not a QA engineer's job to evaluate whether a > particular memory leak is serious or not. > > A good QA automation will thus use 'valgrind --show-reachable=yes' > and the developer's suppression files. > >
GNU values choice for the user. Perhaps GNU should provide a configuration option --enable-memory-leaks for those who want them. The rest of the world can enjoy a clean module that can pass through code quality and security gates. Jeff