svn commit: r261080 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern

[Bruce Evans]

On Thu, 23 Jan 2014, Konstantin Belousov wrote:

> Author: kib
>
> Log:
>  The posix_fallocate(2) syscall should return error number on error,
>  without modifying errno.
>
>  Reported and tested by:	Gennady Proskurin <gpr at mail.ru>
>  Reviewed by:	mdf

This needs many more fixes.  I pointed out some in my review.  Now the
grammar is further unimproved.

> Modified: head/lib/libc/sys/posix_fallocate.2
> ==============================================================================
> --- head/lib/libc/sys/posix_fallocate.2	Thu Jan 23 14:13:12 2014	(r261079)
> +++ head/lib/libc/sys/posix_fallocate.2	Thu Jan 23 17:24:26 2014	(r261080)
> @@ -83,9 +83,9 @@ that reduces the file size to a size sma
> If successful,
> .Fn posix_fallocate
> returns zero.
> -It returns -1 on failure, and sets
> +It returns error number on failure, without setting

New bug (grammar error): missing article before "error number".  The previous
version said plain "error".  The grammar error is not as large for that,
and could have been fixed by saying "an error".  None of "error", "error
number", "an error", "the error", "an error number" or "the error number"
describes what the error actually is.  "an error" improves the wording of
the previous version while keeping its fuzziness.

> .Va errno
> -to indicate the error.
> +variable.

Old bugs (new in the previous version):
1. (grammar error): missing article before "errno variable".
2. (verbosness): after fixing the grammar error, "the errno variable" is
a verbose and unusual way of saying "errno".  Most man pages just say
"errno", like this one used to do before "variable" was added.

Grepping for grammar near errno in libc/sys/*.2 shows a few more grammar
errors and mounds of style bugs:
- 3 man pages use .Dv instead of the usual .Va.  mdoc(7) says that .Va is
   for generic variables and .Dv is for variables (sic) or constants defined
   (sic) in an include file.  The latter makes little sense, and all the
   examples are for manifest (#defined) constants.  Every variable that is
   worth documenting in a man page should be declared, but never defined
   in an include file.
- wording for the style bug of not using -std is inconsistent even within
   aio man pages.  Some say that errno is "set appropriately: and others say
   that errno is set "to indicate the error condition" (but aio has complications
   so that -std is often not usable).  Another says "as enumerated below",
   where the enumeration is in a normal ERRORS section.
- one aio man page quotes -1
- brk.2 "the global variable" errno.  This is not just verbose, but is wrong
   in the threaded case.
- cap_ioctls_limit.2 abuses .Va for -1 and doesn't mark up errno at all.  It
   also says "the global variable errno".
- clock_gettime.2 uses -std, but has an unusual ERRORS section that begins
   with "The following error codes may be set in errno".  If that is correct,
   then the error handling isn't fully -std.  But I think it is -std.  Normal
   wording is for POSIX to say "shall fail if" and ERRORS to say "will fail
   if"
     (I don't like the "will" wording.  deshallify.sh never changes
     "shall" to "will".  It mostly either just removes "shall", or
     changes it to "is" or "are".  This gives better wording, but might
     not be suitable for the ERRORS section).
   The "may be set" wording says very little.
- the cpuset man pages copy the bad example set in clock_gettime.2
- dup.2 says that errno is an "external" variable.  It is missing the "will"
   bug, and just says "fails" instead of "will fail".
- intro.2 says both "the external identifer errno" (and gives excessive
   detail about the implementation of this) and "the variable errno".  It
   gives some details about syscalls setting errno.  These details are now
   wrong for syscalls like posix_fallocate().
- mlockall.2 says "the global location errno"
- modfind.2 says both "sets errno to indicate the error" and "errno is set
   to indicate the error".  It uses the former where it should use -std to
   give the latter (or whatever), and uses the latter in the ERRORS section
   where it shouldn't mention errno again.
- modnext.2 has an unusual (non-bulleted) ERRORS section.
- another variation is "sets the global variable errno to indicate the error"
- msgrcv.2 has the grammar error "and errno set to E*" in several places
- nanosleep.2 says "the global variable errno will be set to indicate the
   interruption"
- nfssvc.2 in some places says "errno == E*" instead of something involving
   errno being set.  In other places it says "the global variable errno is
   set to specify the error" (the -std wording is "indicate", not "specify")
- posix_openpt.2 says "-1 shall be returned and errno set to indicate the
   error".  It hasn't been deshallified.
- procctl.2 spells -1 verbosely as "a value of -1"
- read.2 spells -1 as "a -1"
- readlink.2 spells "sets errno" as "placing the error code in the global
   variable errno"
- reboot.2 says "an error [sic] is returned [sic] in the global variable
   errno
- sched_get_priority_max.2 says "if unsucessful, they shall return a value
   of -1 and set errno" and also "On failure, errno will be set"
- syscall.2 says "an error code is stored in errno"
- The above mostly only mentions the first instance of each style bug.
   There are very few instances of "errno" that don't involve a style bug
   that is a little larger than not using -std.  "the global variable errno"
   is popular, and is what is given by -std.  -std also gives the verboseness
   "the value 0" and "the value -1".

All this variation in an error where there is a macro designed to prevent
it and to allow changing the wording easily :-(.

Bruce