Date: Sat, 25 Jan 2014 06:46:16 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Gennady Proskurin <gpr@mail.ru>, mdf@freebsd.org, standards@freebsd.org, jhb@freebsd.org Subject: Re: standards/186028: incorrect return values for posix_fallocate() Message-ID: <20140125062048.R1518@besplex.bde.org> In-Reply-To: <20140124181246.GO24664@kib.kiev.ua> References: <201401230858.s0N8wwQB039907@oldred.freebsd.org> <20140123094017.GH24664@kib.kiev.ua> <F5BBA557-7ACA-4A7A-9245-165CF962922D@gid.co.uk> <20140124181246.GO24664@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 24 Jan 2014, Konstantin Belousov wrote:
> On Thu, Jan 23, 2014 at 09:20:11PM +0000, Bob Bishop wrote:
>> FWIW at least posix_madvise() and posix_fadvise() have the same problem.
>
> Indeed, I suppose the following should also fix two syscalls noted.
> I hope that some doc committer would do the pass over man pages
> following Bruce' suggestions.
>
> diff --git a/lib/libc/gen/pmadvise.c b/lib/libc/gen/pmadvise.c
> index 60cef63..68b1181 100644
> --- a/lib/libc/gen/pmadvise.c
> +++ b/lib/libc/gen/pmadvise.c
> @@ -12,5 +12,14 @@ __FBSDID("$FreeBSD$");
> int
> posix_madvise(void *address, size_t size, int how)
> {
> - return madvise(address, size, how);
> + int ret, saved_errno;
> +
> + saved_errno = errno;
> + if (madvise(address, size, how) == -1) {
> + ret = errno;
> + errno = saved_errno;
> + } else {
> + ret = 0;
> + }
> + return (ret);
> }
I think it is better to not preserve errno, but to intentionally clobber
it to the same value as madvise() does. Its value is unspecified, and
it is better to set it to new garbage than old garbage in case the
application doesn't understand POSIX's suprising error handling. This
follows from C's specification of errno and POSIX's not saying anything
about errnor for posix_madvise(). From a C99 draft:
[#3] The value of errno is zero at program startup, but is
never set to zero by any library function.159) The value of
errno may be set to nonzero by a library function call
whether or not there is an error, provided the use of errno
is not documented in the description of the function in this
International Standard.
So any library function can set errno to any nonzero value other than
0, unless it is specifically documented to not do so. stdio
(__smakebuf()) setting errno to ENOTTY on success in most cases (by
calling isatty() on a non-terminal) a classic example, and
posix_madvise() should be no different, with more reason than stdio.
In the above, you preserve errno on failure but don't explicitly
preserve it on sucess. madvise() doesn't clobber it on success, but
this is an implementation detail. For most functions, errno becomes
garbage on success.
In POSIX, isatty() is permitted but not required to set errno on success.
stdio is not special except that it is required to set errno to a
specific value on certain errors and required to set errno "to indicate
the error" for all errors. (I only checked fopen() here. I forgot that
ENOTTY tends to occur later in FreeBSD stdio due to delayed buffer
allocation. The buffer allocation optimizations are mostly pessimizations
for both space and time due to bloat in libc. Static allocation of huge
buffers would lose less.)
> diff --git a/lib/libc/sys/madvise.2 b/lib/libc/sys/madvise.2
> index b5ea6b2..755ecc2 100644
> --- a/lib/libc/sys/madvise.2
> +++ b/lib/libc/sys/madvise.2
> @@ -50,7 +50,10 @@ allows a process that has knowledge of its memory behavior
> to describe it to the system.
> The
> .Fn posix_madvise
> -interface is identical and is provided for standards conformance.
> +interface is identical, except it returns an error number on error and does
> +not modify
> +.Va errno ,
> +and is provided for standards conformance.
> .Pp
> The known behaviors are:
> .Bl -tag -width MADV_SEQUENTIAL
The man pages emphasize not setting errno too much. This matches your
implementation (assuming the undocmented behaviour that madvise() doesn't
modify errno on success), but applications shouldn't depend on this.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140125062048.R1518>