svn commit: r344451 - head/usr.sbin/nfsd

Bruce Evans brde at optusnet.com.au
Fri Feb 22 04:26:22 UTC 2019


On Thu, 21 Feb 2019, Sean Eric Fagan wrote:

> Log:
>  Fix the usage error I introduced in r344192.
>
>  Specifically, I put the new option line in the wrong place, and then fixed
>  up the rest without realizing it.  This puts the usage statement back to
>  what it was, with an additional line for the new -V option.

This restores the misformatting of the output.

Usage messages should have the same formatting as the man page.  Not to
mention they should have the same options.

> Modified: head/usr.sbin/nfsd/nfsd.c
> ==============================================================================
> --- head/usr.sbin/nfsd/nfsd.c	Thu Feb 21 22:49:21 2019	(r344450)
> +++ head/usr.sbin/nfsd/nfsd.c	Thu Feb 21 22:49:39 2019	(r344451)
> @@ -189,9 +189,9 @@ main(int argc, char **argv)
> 	    "usage:\n"
> 	    "  nfsd [-ardtue] [-h bindip]\n"

Misformatting starts here.  man(1) output has a 5-column left margin
without "usage: ".  Normal usage messages use a 7-column left margin
as needed to fit "usage: " before the command name on the same line.

> 	    "       [-n numservers] [--minthreads #] [--maxthreads #]\n"
> -	    "       [-p/--pnfs dsserver0:/dsserver0-mounted-on-dir,...,\n"
> -	    "       [-V virtual_hostname]\n"
> -	    "       dsserverN:/dsserverN-mounted-on-dir] [-m mirrorlevel]\n";
> +	    "       [-p/--pnfs dsserver0:/dsserver0-mounted-on-dir,...,"
> +	    "dsserverN:/dsserverN-mounted-on-dir] [-m mirrorlevel]\n"

This is the worst misformatting.

> +	    "       [-V virtual_hostname]\n";
> 	while ((ch = getopt_long(argc, argv, getopt_shortopts, longopts,
> 		    &longindex)) != -1)
> 		switch (ch) {

The man(1) output is:

XX      nfsd [-ardute] [-n num_servers] [-h bindip] [-p pnfs_setup]
XX           [-m mirror_level] [-V virtual_hostname] [--maxthreads max_threads]
XX           [--minthreads min_threads]

This is missing the --pnfs and -V options.

The usage messge is:

XX usage:
XX   nfsd [-ardtue] [-h bindip]
XX        [-n numservers] [--minthreads #] [--maxthreads #]
XX        [-p/--pnfs dsserver0:/dsserver0-mounted-on-dir,...,dsserverN:/dsserverN-mounted-on-dir] [-m mirrorlevel]
XX        [-V virtual_hostname]

Other bugs in the usage message:
- the -n option is unsorted
- the name of the arg for the -n option is different
- the --minthreads and --maxthreads options are sorted backwards
- the name of the arg for --minthreads and --maxthreads is different and
   unusual.  Man pages cannot use the abbreviation '#' for a number since
   this is not unique.
- the arg for the -p/--pnfs option is a description, not a name.  This is
   also unsuitable for the synopsis in man pages
- the description of the arg for the -p/--pnfs option is too long.  man(1)
   would make a different mess of formatting a single long token.
- the description for -p/-pnfs is not normally formatted.  cp(1) gave an
   example of normal formatting for lists, but this was and is broken too.
   It was "src1 ... srcN directory" and is now "source_file ... target
   directory".  The old version is apparently carefully abbreviated to fit
   on 1 line, but " srcN" is redundant if N > 1 and a syntax error if
   N == 1.  nfsd has the same syntax error and this helps mess up the
   formatting.  cp leaves a space between the arg names and the ellipsis.
   The commas in the nfsd arg are needed to give a single token, but are
   especially hard to format and to describe in a more formal syntax.
   nfsd's man page doesn't attempt to give a more formal syntax.  The
   current version of cp(1) fixes the redundant/wrong srcN, but expands
   the arg names and misformats the resulting long line only in the usage
   message.  The separate tokens allow man(1) to split the line at a bad
   place, but it somehow splits early so as to not split in the middle of
   related tokens.

   I found misformatting in man output in additions to vidcontrol(1)
   yesterday.  It needs to start a new line for the -f arg after adding
   args before -f, but splits in between the option and the arg.  This
   seems to be because the -f option has a complicated fairly formal
   syntax involving lots of square brackets and the man page source uses
   lots of markup to express this, and this confuses man(1).

- there isn't even a newline after the long description for -p/--pnfs
   gives a too-long line
- the -m option is unsorted
- the name of the arg for the -m option is different.

I seem to have lost my synopsis/usage checker.  Here is a quick
re-implementation:

XX #!/bin/sh
XX 
XX name=$1
XX 
XX IFS= synopsis=$(man $name | col -bx | awk '{
XX 	if (match($1, "SYNOPSIS")) {
XX 		state = 1
XX 		next
XX 	} else if (state == 0)
XX 		next
XX 	else if (length($0) == 0)
XX 		exit
XX 	if (state == 1) {
XX 		print "usage: " substr($0, 6)
XX 		state = 2
XX 	} else
XX 		print "  " $0
XX }')
XX IFS= usage=$($name -? --help 2>&1 <>/dev/null | grep -v "illegal option")
XX 
XX if test "$synopsis" = "$usage"; then
XX 	echo "$name passed synopsis vs usage check"
XX else
XX 	echo "$name failed synopsis vs usage check"
XX 	# XXX: should do a diff here.
XX 	echo "synopsis:"
XX 	echo "$synopsis"
XX 	echo "usage:"
XX 	echo "$usage"
XX fi

A reasonable number of files pass the strict lexical identity check (after
minor edits to man(1) output).  Pass/fail counts:

                FreeBSD-~5.2    FreeBSD-12.0-ALPHA10
/bin           11/32           15/28   # some improvements
/sbin          20/78           20/116  # no improvements
/usr/bin       # failed (bugware doesn't check options and hangs)
/usr/sbin      # failed (bugware doesn't check options and hangs)
/usr/libexec   # failed (bugware as above, or man(1) spins on bad man page)

Just found the old version.  It uses sed instead of awk and edits the usage
method instead of the man page, and removes 1 more line in the usage:

XX 	sed -e '/Unrecognized switch/d' \

but is otherwise worse.

Bruce


More information about the svn-src-all mailing list