From owner-svn-src-head@FreeBSD.ORG Thu Jan 23 23:56:28 2014 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2C904D78; Thu, 23 Jan 2014 23:56:28 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id CDAEB1AB8; Thu, 23 Jan 2014 23:56:27 +0000 (UTC) Received: from c122-106-144-87.carlnfd1.nsw.optusnet.com.au (c122-106-144-87.carlnfd1.nsw.optusnet.com.au [122.106.144.87]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 9E9131A29FE; Fri, 24 Jan 2014 10:56:17 +1100 (EST) Date: Fri, 24 Jan 2014 10:56:16 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov Subject: Re: svn commit: r261080 - in head: lib/libc/sys sys/compat/freebsd32 sys/kern In-Reply-To: <201401231724.s0NHOQLE039584@svn.freebsd.org> Message-ID: <20140124091907.I2614@besplex.bde.org> References: <201401231724.s0NHOQLE039584@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=bpB1Wiqi c=1 sm=1 tr=0 a=p/w0leo876FR0WNmYI1KeA==:117 a=PO7r1zJSAAAA:8 a=aMK5myg-p9oA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=Nhj2jhtrgT8A:10 a=RgBw9RmQAAAA:8 a=mBz_84EukJEfn65XYgIA:9 a=jr8VamtQr4TqBslQ:21 a=C4T51DSm5XGUXB0p:21 a=CjuIK1q_8ugA:10 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Jan 2014 23:56:28 -0000 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 > 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