Paul Eggert wrote: > On 08/28/2010 09:26 AM, Jim Meyering wrote: > >> This bug was introduced by commit be6c13e7, "maint: always free a >> buffer, to avoid even semblance of a leak". > > Hah! I've always been suspicious of these unnecessary free()s, > i.e., free()s that are put in only to make 'valgrind' happy. > > How about if we do these unnecessary free()s only if 'lint' > is defined? That would make the production software more > reliable.
Hi Paul, It's a close call, especially given this actual bug. However, for me at least, this is similar to compiler warnings in that freeing unconditionally makes for cleaner output from leak-checking tools like valgrind, and thus, desirable. In addition, guarding the free inside #ifdef lint...#endif would expose the "possibly leaked" condition to any unsuspecting developer who builds without -Dlint. We've had a few reports like that: the solution is simply to tell such folks to use -Dlint, but preventing reports altogether is even better. Another point against such a change is that it'd add an in-function #ifdef. On the other hand, there is precedent for this sort of a guard in e.g., shuf.c and mktemp.c. In addition, when configuring with --enable-gcc-warnings, you automatically get -Dlint, so maybe it makes sense. I'm tempted to push this, but could go either way: >From 9e837201ba6d8974de8e09c1d359c16ecab12584 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sat, 28 Aug 2010 21:54:17 +0200 Subject: [PATCH] tac: suppress technically unneeded "free" * src/tac.c (main): Guard final free with #ifdef lint. Suggested by Paul Eggert. --- src/tac.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/tac.c b/src/tac.c index 859e006..dbfe2b0 100644 --- a/src/tac.c +++ b/src/tac.c @@ -666,8 +666,10 @@ main (int argc, char **argv) ok = false; } +#ifdef lint size_t offset = sentinel_length ? sentinel_length : 1; free (G_buffer - offset); +#endif exit (ok ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.2.2.510.g7180a
