svn commit: r229937 - in head/lib: libc/gen libutil

Bruce Evans brde at optusnet.com.au
Wed Jan 11 16:40:08 UTC 2012


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


More information about the svn-src-all mailing list