From owner-svn-src-all@FreeBSD.ORG Wed Jan 11 16:40:08 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 44CC0106564A; Wed, 11 Jan 2012 16:40:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id CAAFA8FC18; Wed, 11 Jan 2012 16:40:07 +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 mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q0BGe3Vg010012 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 12 Jan 2012 03:40:05 +1100 Date: Thu, 12 Jan 2012 03:40:03 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Guy Helmer In-Reply-To: <26EF49B4-1B8E-43BC-AC7B-FD24BB94C961@palisadesystems.com> Message-ID: <20120112025133.H2744@besplex.bde.org> References: <201201101953.q0AJrPao025097@svn.freebsd.org> <20120110221323.GE1694@garage.freebsd.pl> <20120111214848.K1813@besplex.bde.org> <26EF49B4-1B8E-43BC-AC7B-FD24BB94C961@palisadesystems.com> 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, Pawel Jakub Dawidek , Bruce Evans 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 16:40:08 -0000 On Wed, 11 Jan 2012, Guy Helmer wrote: > On Jan 11, 2012, at 6:25 AM, Bruce Evans wrote: >> 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 >> ... The Makefile for libutil is also unsorted: - SRCS is sorted or nearly so - disorder in MAN begins with pty.3. The rest of MAN is unsorted in much the same disorder as the prototypes - MLINKS is considerably more disordered, but is much larger so it needs ordering even more. SRCS and MAN are simple lists (except for the internal structure of MAN -- sections). MLINKS has more internal structure (pairs of link source and target, which should be sorted on the source first, then the target. See Makefile.inc's in libc for rasonably correct examples. > Does this patch improve the situation? Sure. > I have not found guidance for line lengths in style(9) -- am I looking in the wrong place? Not really. style(9) is supposed to consist mainly of examples. It contains no examples of lines longer than 80 (preferably 79). Therefore, lines longer than 80 are bad style. You have to know that it has been mangled into man page form and cancel the lossages from this. One lossage is that man adds a left margin of 5 spaces. So it is lines of length longer than 85 in the man output that would be too long. man should format man pages for the terminal or page width. This might exceed 80, or it might be less than 85 so it might be man and not the examples that are giving the line length limit. However, most of the examples are in literal text which man should not reformat. Thus the output has some lines slightly longer than 80 if that is the terminal width, but hopefully none more than 85. The 5 character margin also destroys the tab structure in the output. The man page is careful to preserve the 8-column primary indentation and the 4-column continuation-line indentation, but these cannot be done with tabs. Some user like wide terminals, but small screens are more common now, so even 80 columns may be too wide for a default. It's just too hard to have enough indentation in 64 columns. > Index: libutil.h > =================================================================== > --- libutil.h (revision 229961) > +++ libutil.h (working copy) > @@ -84,110 +84,117 @@ > ... > __BEGIN_DECLS > -void clean_environment(const char * const *_white, > +char *auth_getval(const char *_name); > +void clean_environment(const char * const *_white, > const char * const *_more_white); > -int extattr_namespace_to_string(int _attrnamespace, char **_string); > -int extattr_string_to_namespace(const char *_string, int *_attrnamespace); > -int flopen(const char *_path, int _flags, ...); I'd prefer to see the changes separately: sort, reformat, fix namespaces; not necessarily in that order. Sorting gives especially unreadable diffs. Here you changed to the style that leaves an extra space before the function name to line up with function names that have a `*' before them. That is too much for me. It also gives unreadable diffs by changing almost every line, since it was not used in this file before. This can be seen in the declaration of `clean_environment' above. > ... > +int uu_unlock(const char *_ttyname); > +int uu_lock_txfr(const char *_ttyname, pid_t _pid); > +int _secure_path(const char *_path, uid_t _uid, gid_t _gid); The best order for sorting `_foo' is unclear. In ASCII it is between A-Z and a-z. Lists in makefiles mostly use pure ASCII order since that is easiest to generate and check by doing the sorting using ls. In source code we might prefer to ignore underscores in sorting. The leading underscore in this name seems to be bogus. The function is documented. Use of it in other library functions might require an underscore to avoid namespace pollution, but this library is not generally careful about that. > > + Extra blank line. > +int quota_check_path(const struct quotafile *_qf, const char *_path); > +void quota_close(struct quotafile *_qf); > +int quota_convert(struct quotafile *_qf, int _wordsize); > ... I checked that a few of these names match the man page. Seems fairly complete and correct. Bruce