From owner-svn-src-all@freebsd.org Mon May 30 22:41:29 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7E6CCB4FC70; Mon, 30 May 2016 22:41:29 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (unknown [IPv6:2602:304:b010:ef20::f2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gw.catspoiler.org", Issuer "gw.catspoiler.org" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4328D1A46; Mon, 30 May 2016 22:41:29 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.15.2/8.15.2) with ESMTP id u4UMfIWZ029756; Mon, 30 May 2016 15:41:22 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <201605302241.u4UMfIWZ029756@gw.catspoiler.org> Date: Mon, 30 May 2016 15:41:18 -0700 (PDT) From: Don Lewis Subject: Re: svn commit: r300952 - head/usr.sbin/services_mkdb To: asomers@freebsd.org cc: ed@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org In-Reply-To: MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 May 2016 22:41:29 -0000 On 29 May, Alan Somers wrote: > On Sun, May 29, 2016 at 4:41 AM, Ed Schouten 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.