Date: Wed, 11 Jan 2012 23:25:57 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Pawel Jakub Dawidek <pjd@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Guy Helmer <ghelmer@freebsd.org> Subject: Re: svn commit: r229937 - in head/lib: libc/gen libutil Message-ID: <20120111214848.K1813@besplex.bde.org> In-Reply-To: <20120110221323.GE1694@garage.freebsd.pl> References: <201201101953.q0AJrPao025097@svn.freebsd.org> <20120110221323.GE1694@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120111214848.K1813>