From owner-svn-src-all@FreeBSD.ORG Wed Jan 11 12:26:02 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3B283106566C; Wed, 11 Jan 2012 12:26:02 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail08.syd.optusnet.com.au (mail08.syd.optusnet.com.au [211.29.132.189]) by mx1.freebsd.org (Postfix) with ESMTP id A622E8FC16; Wed, 11 Jan 2012 12:26:01 +0000 (UTC) Received: from c211-30-171-136.carlnfd1.nsw.optusnet.com.au (c211-30-171-136.carlnfd1.nsw.optusnet.com.au [211.30.171.136]) by mail08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q0BCPv2J013153 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 11 Jan 2012 23:25:58 +1100 Date: Wed, 11 Jan 2012 23:25:57 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Pawel Jakub Dawidek In-Reply-To: <20120110221323.GE1694@garage.freebsd.pl> Message-ID: <20120111214848.K1813@besplex.bde.org> References: <201201101953.q0AJrPao025097@svn.freebsd.org> <20120110221323.GE1694@garage.freebsd.pl> 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, Guy Helmer Subject: Re: svn commit: r229937 - in head/lib: libc/gen libutil X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Wed, 11 Jan 2012 12:26:02 -0000 On Tue, 10 Jan 2012, Pawel Jakub Dawidek wrote: > On Tue, Jan 10, 2012 at 07:53:25PM +0000, Guy Helmer wrote: > [...] >> Modified: head/lib/libutil/libutil.h >> ============================================================================== >> --- head/lib/libutil/libutil.h Tue Jan 10 18:43:27 2012 (r229936) >> +++ head/lib/libutil/libutil.h Tue Jan 10 19:53:25 2012 (r229937) >> @@ -170,6 +170,7 @@ struct pidfh *pidfile_open(const char *p >> int pidfile_write(struct pidfh *pfh); >> int pidfile_close(struct pidfh *pfh); >> int pidfile_remove(struct pidfh *pfh); >> +int pidfile_fileno(struct pidfh *pfh); >> #endif >> >> #ifdef _UFS_UFS_QUOTA_H_ > > One more thing. You also need to add link in Makefile, so that > 'man pidfile_fileno' will work. Is there any chance of keeping sorted lists sorted? This file used to be mostly sorted and mostly in KNF (tab before function names) and mostly without namespace pollution in parameter names). Newer code in it violates all of these style and header implementation rules. The pidfile code was already especially bad, and adding to the end of the unsorted list in it doesn't help. The pidfile man page is also unsorted. It was in "operations" order "open/write/close/remove". That is hard to maintain and hard to search for long lists. Adding pidfile_fileno() to the end of the list in the same disorder as above makes the list not even in "operations" order (since pidfile_fileno() is only valid betwen pidfile_open() and pidfile_close()). Old disorder and nearby bugs in libutil.h (old := before 2005): - the forward declarations of structs are totally disordered - the following prototypes are disordered (mostly by adding them to the end of a orginally-almost-sorted list: trimdomain(), forkpty(), humanize_number(), uu_lockerr(), uu_unlock(), _secure_path(), properties_read(), properties_free(), properties_find(), auth_getval(), realhostname(), - the forward declaration of struct sockaddr is disorderd (not unsorted at the beginning with the others) - surprisingly, realhostname_sa() is not unsorted relative to realhostname() - the following prototypes have namespace pollution in parameter names (their parameter names are in the implementation namespace, unlike for all of the older prototypes in the file): properties read(), properties free(), properties free(), auth_getval(), realhostname(), realhostname_sa(). The properties code used to be the only really ugly code in this file. - formatting errors for openpty() (premature line splitting and wrong continuation indent). The very first prototype in this file gives an example of normal splitting and continuation indent - formatting errors for forkpty() (same as for openpty()) - line too long for realhostname_sa(), pw_copy() - consider fparseln() as being in a new section so it isn't unsorted. The start of this section should be delimited by a blank line (this is done in -current). The new section isn't really justified. It is just 1 function ifdefed to avoid a namespace problem. - no namespace pollution for fparseln()'s parameters, but that it because it is in a style different from all older prototypes in the file -- it doesn't name its parameters. Newer disorder and nearby bugs: - further unsorting of the forward declarations by adding kinfo* to the end of the unsorted list. At least the new declarations are sorted internally. - the following protypes in the early sections are disordered: expand_number(), kld*(), kinfo* (no need for a new section for the last 2. At least they are each sorted internally), gr_scan() - the following prototypes have namespace pollution in parameter names: hexdump(), kld*(), gr_dup(), gr_equal(), gr_make(), gr_scan(). Only about half of the gr*() prototypes have this bug - better indentation for kinfo*() than for older or newer entries! - line too long for gr_entry() - pidfile*() have all of the above bugs, plus they are the only prototypes up to this point in the file that don't put a tab before the function name when the return type is int - quota*() are slightly worse than pidfile*(). They add the additional style bug of not using parameter names at all, except for 1 of 2 parameters in 1 of 12 prototypes. That parameter is namespace pollution. Bruce