Re: musl, fdopen test

[Bruno Haible <bruno@clisp.org>]

Rich Felker wrote:
> > >> Replacement of fdopen, because of
> > >>   checking whether fdopen sets errno... no
> > > 
> > > There was one bug here (failure to set errno when mode string was
> > > invalid) but I don't think that's the case gnulib was testing for. It
> > > seems gnulib wants an error for the "may fail" when the fd is invalid.

Indeed, the possibility that fdopen(invalid fd, ...) can succeed is supported
by POSIX. When looking at the gnulib documentation
  doc/posix-functions/fdopen.texi
and the unit test
  tests/test-fdopen.c
it appears that it was not the intent of gnulib to prohibit this behaviour.
Rather, musl is the first platform to exhibit this behaviour, and gnulib's
intent was to make sure that fdopen(invalid fd, ...)
  1. does not crash,
  2. sets errno when it fails.

Eric Blake replied:
> > The 'EBADF may fail' condition is rather weak.  And since glibc
> > guarantees a definite failure, it is nicer to program to the glibc
> > interface that guarantees immediate failure on a bad fd at fdopen() time
> > than it is to deal with the surprises that result when fdopen() succeeds
> > but later attempts to use the stream fail.  Perhaps it might be worth

The glibc documentation contains this warning:

     In some other systems, `fdopen' may fail to detect that the modes
     for file descriptor do not permit the access specified by
     `opentype'.  The GNU C library always checks for this.

So, I think few programmers will explicitly want to exploit this glibc
specific behaviour.

Rich Felker replied:
> The only real-world situation I can think of where you'd care that
> fdopen detect EBADF is when you've just called a function that
> allocates the file descriptor and directly passed it to fdopen without
> first checking the return value. For instance:
> 
> FILE *f = fdopen(open(pathname, O_RDWR|O_CLOEXEC), "rb+");
> 
> instead of:
> 
> int fd = open(pathname, O_RDWR|O_CLOEXEC);
> if (fd<0) goto error;
> FILE *f = fdopen(fd, "rb+");
> 
> The former is rather lazy programming, but maybe gnulib intends to
> support this kind of programming.

No, gnulib does not intend to encourage this kind of lazy programming.

> My thought in having musl skip the test is to maximize performance of
> fdopen, assuming you might be using it in a situation like on a newly
> accept()ed network connection where every syscall counts (think
> multi-threaded httpd). For read-only fdopen, no syscalls are needed,

Sounds reasonable.

Here's a proposed patch to remove gnulib's unintentional requirement.


2012-06-19  Bruno Haible  <bruno@clisp.org>

	fdopen: Allow implementations that don't reject invalid fd arguments.
	* m4/fdopen.m4 (gl_FUNC_FDOPEN): Let the test pass if fdopen(-1,...)
	succeeds.
	Reported by Rich Felker <dalias@aerifal.cx>.

--- m4/fdopen.m4.orig	Tue Jun 19 13:00:23 2012
+++ m4/fdopen.m4	Tue Jun 19 13:00:05 2012
@@ -1,4 +1,4 @@
-# fdopen.m4 serial 2
+# fdopen.m4 serial 3
 dnl Copyright (C) 2011-2012 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -25,10 +25,8 @@
   FILE *fp;
   errno = 0;
   fp = fdopen (-1, "r");
-  if (fp != NULL)
+  if (fp == NULL && errno == 0)
     return 1;
-  if (errno == 0)
-    return 2;
   return 0;
 }]])],
           [gl_cv_func_fdopen_works=yes],