From owner-svn-src-head@FreeBSD.ORG Wed Jun 20 16:27:28 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5A8D61065672; Wed, 20 Jun 2012 16:27:28 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.freebsd.org (Postfix) with ESMTP id E2ECC8FC15; Wed, 20 Jun 2012 16:27:27 +0000 (UTC) Received: from c122-106-171-232.carlnfd1.nsw.optusnet.com.au (c122-106-171-232.carlnfd1.nsw.optusnet.com.au [122.106.171.232]) by mail04.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q5KGRNTt028595 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 21 Jun 2012 02:27:25 +1000 Date: Thu, 21 Jun 2012 02:27:23 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler In-Reply-To: <201206200638.q5K6cg7u024024@svn.freebsd.org> Message-ID: <20120621015220.J2636@besplex.bde.org> References: <201206200638.q5K6cg7u024024@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r237286 - head/lib/libc/gen X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Jun 2012 16:27:28 -0000 On Wed, 20 Jun 2012, Eitan Adler wrote: > Log: > Don't close an uninitialized descriptor. [1] > Add a sanity check for the validity of the passed fd. Library functions shouldn't use assert() or abort(). The fd is not passed, but is a static variable under syslog()'s control. In libc, only the following use assert.h: - stdio/xprintf*.c. Very nonstd. - rpc/*.c (perhaps not everything) - db/*.c (but partly under DEBUG. Not the standard use where assert() is controlled by NDEBUG) - nameser/ns_print.c - ia64/gen/unwind.c (but under SANITY. The standard NDEBUG is too hard to use here too) - gen/getgrent.c. - regex/grot/main.c (test program. Not part of libc) - regex/regex/utils.h (but ifdefed, and I think turned off in production. Spencer knows how to use NDEBUG) - regex/regcomp.c (ifdefed) - include/isc/list.h - net/*.c (just 2 files) - posix1e/acl*.c (perhaps not everything) - inet/inet_net_pton.c - iconv/*.c (perhaps not everything). The list was only short enough to be described by "only" in not very old versions of FreeBSD. posix1e and iconv doubled the number of files. > Modified: head/lib/libc/gen/syslog.c > ============================================================================== > --- head/lib/libc/gen/syslog.c Wed Jun 20 04:11:34 2012 (r237285) > +++ head/lib/libc/gen/syslog.c Wed Jun 20 06:38:41 2012 (r237286) > @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > > +#include > #include > #include > #include > @@ -413,8 +414,11 @@ void > closelog(void) > { > THREAD_LOCK(); > - (void)_close(LogFile); > - LogFile = -1; > + assert(LogFile >= -1); Since this assert() can't fail (unless the CPU has a parity error or or the memory behind Logfile has a parity error or another memory error or was clobbered by a buffer overrun), this assert() doesn't break the library but just wastes space. > + if (LogFile != -1) { > + (void)_close(LogFile); > + LogFile = -1; > + } > LogTag = NULL; > status = NOCONN; > THREAD_UNLOCK(); Bruce