Joey Degges wrote: > On Tue, Feb 16, 2010 at 5:05 AM, Eric Blake <e...@byu.net> wrote: > >> According to Joey Degges on 2/16/2010 2:02 AM: >> > Hello, >> > >> > At sort.c:3271 'files' is allocated but it is not free'd before main >> exits: >> > files = xnmalloc (argc, sizeof *files); >> >> Thanks for the patches. However, calling free() immediately before exit() >> is a lint-like activity - it is actually SLOWER to explicitly free memory >> rather than just exiting and letting the OS cleanup reclaim the memory as >> part of process death. If we accept patches like this, it will be to make >> other leak detections easier, but as such, it should probably be properly >> guarded by #if LINT or something similar to make it apparent that it is >> only needed when looking for leaks and not in the common case. > > > Thanks for your insight -- I was not aware of 'lint' before. I have > reformatted the patch with #ifdef lint so that this will only be used if > gcc-warnings is enabled. If this looks good I will also resubmit the other > two patches. > >>From 0018a314269bc8a9b89e82be2cbf17a08d28f297 Mon Sep 17 00:00:00 2001 > From: Joey Degges <jdeg...@gmail.com> > Date: Mon, 15 Feb 2010 23:30:31 -0800 > Subject: [PATCH 1/3] sort.c: Fix minor memory leak, 'files' is never free'd
Thanks for caring, but at least in my book, this is not even a "minor leak". A more apt one-line summary would be "sort: free memory allocated in main" > src/sort.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/src/sort.c b/src/sort.c > index 481fdb8..a2cba05 100644 > --- a/src/sort.c > +++ b/src/sort.c > @@ -3692,6 +3692,11 @@ main (int argc, char **argv) > else > sort (files, nfiles, outfile); > > +#ifdef lint > + if (nfiles != 0) > + free (files); > +#endif Also, the above will malfunction when nfiles is initially 0, since we set file = &minus and "nfiles = 1". With your proposed addition, that would make sort call free(&minus), which would probably segfault. Considering that freeing "files" is not at all important, I think it would be better to tell whatever tool you're using that this particular case is not a problem. In valgrind, you can add a so-called "suppression".
