Date: Mon, 30 May 2016 15:41:18 -0700 (PDT) From: Don Lewis <truckman@FreeBSD.org> To: asomers@freebsd.org Cc: ed@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r300952 - head/usr.sbin/services_mkdb Message-ID: <201605302241.u4UMfIWZ029756@gw.catspoiler.org> In-Reply-To: <CAOtMX2i%2Bbw4Xq3wbrJ5j5LURMP=5JZzu6FQWpYPf9brj7-_a-A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 29 May, Alan Somers wrote: > On Sun, May 29, 2016 at 4:41 AM, Ed Schouten <ed@freebsd.org> wrote: >> Author: ed >> Date: Sun May 29 10:41:27 2016 >> New Revision: 300952 >> URL: https://svnweb.freebsd.org/changeset/base/300952 >> >> Log: >> Invoke the dirname() function in a POSIX compliant way. >> >> POSIX requires that the argument of dirname() is of type "char *". In >> other words, the input buffer can be modified by the function to store >> the directory name. >> >> Pull a copy of the string before calling dirname(). We don't care about >> freeing up the memory afterwards, as this is done at the very bottom of >> main(), right before the program terminates. >> >> Reviewed by: bapt >> Differential Revision: https://reviews.freebsd.org/D6628 >> >> Modified: >> head/usr.sbin/services_mkdb/services_mkdb.c >> >> Modified: head/usr.sbin/services_mkdb/services_mkdb.c >> ============================================================================== >> --- head/usr.sbin/services_mkdb/services_mkdb.c Sun May 29 07:39:56 2016 (r300951) >> +++ head/usr.sbin/services_mkdb/services_mkdb.c Sun May 29 10:41:27 2016 (r300952) >> @@ -92,7 +92,7 @@ main(int argc, char *argv[]) >> size_t cnt = 0; >> StringList *sl, ***svc; >> size_t port, proto; >> - char *dbname_dir; >> + char *dbname_dir, *dbname_dirbuf; >> int dbname_dir_fd = -1; >> >> setprogname(argv[0]); >> @@ -172,7 +172,8 @@ main(int argc, char *argv[]) >> * fsync() to the directory where file lies >> */ >> if (rename(tname, dbname) == -1 || >> - (dbname_dir = dirname(dbname)) == NULL || >> + (dbname_dirbuf = strdup(dbname)) == NULL || >> + (dbname_dir = dirname(dbname_dirbuf)) == NULL || >> (dbname_dir_fd = open(dbname_dir, O_RDONLY|O_DIRECTORY)) == -1 || >> fsync(dbname_dir_fd) != 0) { >> if (dbname_dir_fd != -1) >> > > Even though the program is about to exit, it's worth freeing the > memory just to make Coverity shut up. I usually don't bother in that situation because it's not worth the added code complexity. In Coverity, I'll mark them as a bug, set the severity to insignficant, and set action to ignore. The situation is different if I think the code might get used elsewhere and the memory could be wasted for a longer period of time. I've also run into situations where there is a real downside to doing this sort of cleanup before calling exit(). Generally this is with a process that has allocated some sort of large data structure in memory over a long period of time, especially if the process has grown larger than physical RAM and is partially resident in swap. It's maddening to watch the process page madly for an extended period of time as it chases pointers all of ther place and calls free() zillions of times when it would be so much faster to just call exit(). Adding more RAM may not be an option in such cases, since I've had to endure this on machines that had the maximum amount of RAM that they could hold.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201605302241.u4UMfIWZ029756>