Hi,
Scott Cheloha wrote on Fri, Jul 28, 2017 at 08:33:16PM -0500:
> Unlikely to happen during normal use, but setvbuf(3) can fail
> to allocate your buffer:
>
> /* prog.c */
> #include <stdio.h>
>
> int
> main(int argc, char *argv[])
> {
> if (setvbuf(stdout, NULL, _IOFBF, 0))
> perror("setvbuf");
> return 0;
> }
>
> This seems to force the allocation failure on my box:
>
> $ ksh -c "ulimit -d 230 ; ./prog"
>
> Not sure what the correct wording is for the ERRORS section.
>
> Is it appropriate to specify that you can only get an allocation
> failure under the particular circumstances described? Or is it
> better to just write the standard line?, i.e.
>
> "Insufficient storage space is available."
>
> As a reader I would want to know more precisely how the
> interface can fail. Still seems kind of messy. Maybe my
> wording could be improved.
Your patch is obviosly wrong.
If at all, the required wording would be something like
The setvbuf() function may also fail and set errno for
any of the errors specified for the routine malloc(3).
But that isn't fair game either.
Lets take a step back and look at the big picture, in four steps:
1. The setvbuf(3) function is standardized by POSIX.
POSIX fails to say that setvbuf() sets ENOMEM on allocation
failure, which means that doing so is implementation
dependent, see paragraph 1.2 of
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap01.html
and paragraph 2.3 of
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
In a nutshell, this means that POSIX provisions for error numbers
are mostly advisory because implementations are allowed to differ
in either direction, yet some POSIX provisions *are* mandatory.
Figuring out what exactly is mandatory in each particular case
is tricky.
My impression is that not much care was taken when setting up
the POSIX specifications for error numbers. They are often
obviously incomplete and sometimes unclear.
2. The said, strictly speaking, if including the sentence
shown above into ERRORS, we should also include this sentence
below STANDARDS:
The setvbuf() function conforms to ISO/IEC 9899:1999 ("ISO C99")
and IEEE Std 1003.1-2008 ("POSIX.1").
Setting errno for malloc(3) failure is an extension to
the POSIX standard.
Strictly speaking, both standards must be mentioned, because
not all the behaviour described in the manual is mandated by
C99. For example, setting errno *at all* is already an extension
to the C standard, yet it is mandated by POSIX.
That said, almost none of our manual pages cite two standards
in this way, and practically none properly document which
error numbers are extensions. Given the sloppiness of POSIX
with respect to error numbers, that hardly comes as a surprise.
3. That said, fiddling with these details is petty, there are much
worse problems. For example, the existing ERRORS section in
setvbuf(3) is nothing but one big lie. If the stream is not
associated with a valid file descriptor, setvbuf() will *NOT*
fail:
schwarze@isnote $ cat setvbuf.c
#include <err.h>
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
int
main(void)
{
char buf[16];
FILE *fp;
int fd;
if ((fd = open("testfile.txt", O_RDONLY)) == -1)
err(1, "open");
if ((fp = fdopen(fd, "r")) == NULL)
err(1, "fdopen");
if (close(fd) == -1)
err(1, "close");
if (setvbuf(fp, buf, _IOFBF, 1024) != 0)
err(1, "setvbuf");
if (fgets(buf, sizeof(buf), fp) == NULL)
err(1, "fgets");
return 0;
}
schwarze@isnote $ make setvbuf
cc -O2 -pipe -o setvbuf setvbuf.c
schwarze@isnote $ ./setvbuf
setvbuf: fgets: Bad file descriptor
Note how setvbuf() suceeds and only fgets() fails.
This is *not* a POSIX violation. POSIX only says:
The setvbuf() function may fail if:
[EBADF] The file descriptor underlying stream is not valid.
Yet, this is an outright documentation bug in our manual page.
4. That said, fiddling with even that detail is petty, there are
still worse problems. Consider this program:
schwarze@isnote $ cat setvbuf_invalid.c
#include <err.h>
#include <stdio.h>
int
main(void)
{
FILE *fp;
if ((fp = fopen("testfile.txt", "r")) == NULL)
err(1, "fopen");
if (setvbuf(fp, NULL, _IOFBF, -1) != 0)
err(1, "setvbuf");
return 0;
}
schwarze@isnote $ make setvbuf_invalid
cc -O2 -pipe -o setvbuf_invalid setvbuf_invalid.c
schwarze@isnote $ ./setvbuf_invalid
setvbuf_invalid: setvbuf: Undefined error: 0
A negative buffer size is invalid and causes setvbuf() to
fail, but fails to set errno.
Again, that's not a POSIX violation, POSIX allows additional
implementation-dependent failure scenarios, and POSIX does
not require setting errno.
But arguably, failure to set errno is a bug in our implementation,
because setting errno would clearly be useful, and our manual
even suggests that it might happen.
In any case, our manual is imprecise, neither making it clear
what a programmer trying to write POSIX-conforming, portable
code should expect, nor making it clear what our implementation
actually does.
To summarize, the code is buggy (and almost certainly not only in
the one aspect i demonstrated), our manual is a mess, POSIX
is a mess, too, and you are applying lipstick to a pig.
All this (maybe except POSIX) can be fixed, but that's a huge
project because error handling is very, very messy all across
the C library similar to what i described for this example.
Patches of this kind are not helpful, in particular since you are
clearly not (yet) understanding what you are doing. Checking such
patches causes substantial work for developers and may result in
erractic and inconsistent improvements at best.
If someone has a year of working time to spare to tackle error
handling in libc, it should be done systematically, not in such a
piecemeal manner, and it needs to be done carefully.
Yours,
Ingo
> Index: lib/libc/stdio/setvbuf.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/setvbuf.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 setvbuf.3
> --- lib/libc/stdio/setvbuf.3 26 Nov 2014 18:16:32 -0000 1.4
> +++ lib/libc/stdio/setvbuf.3 29 Jul 2017 01:27:55 -0000
> @@ -138,6 +138,10 @@ function will fail if:
> The
> .Fa stream
> specified is not associated with a valid file descriptor.
> +.It Bq Er ENOMEM
> +Line or block buffering is requested,
> +.Fa buf
> +is NULL, and insufficient storage space is available.
> .El
> .Sh SEE ALSO
> .Xr fclose 3 ,
