Eric Blake wrote:
> --- a/doc/posix-functions/fcntl.texi
> +++ b/doc/posix-functions/fcntl.texi
> @@ -4,10 +4,16 @@ fcntl
>
> POSIX specification: @url
> {http://www.opengroup.org/onlinepubs/9699919799/functions/fcntl.html}
>
> -Gnulib module: ---
> +Gnulib module: fcntl
>
> Portability problems fixed by Gnulib:
> @itemize
> +...@item
> +This function does not support @code{F_DUPFD_CLOEXEC} on some
> +platforms, although the replacement is not atomic:
> +MacOS X 10.3, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, AIX 5.1, HP-UX 11,
> +IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.7.1, mingw, Interix 3.5,
> +BeOS.
The "although" part sounds a bit confusing. How about:
@item
This function does not support @code{F_DUPFD_CLOEXEC} on some
platforms:
MacOS X 10.3, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, AIX 5.1, HP-UX 11,
IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.7.1, mingw, Interix 3.5,
BeOS.
Note that the gnulib replacement code is functional but not atomic.
> + F_DUPFD - duplicate FD, with ACTION being the minimum target fd.
Here you mean ARG, not ACTION.
> + F_DUPFD_CLOEXEC - duplicate FD, with ACTION being the minimum
Likewise.
> +int
> +rpl_fcntl (int fd, int action, ...)
> +{
> + va_list arg;
> + int result = -1;
> + va_start (arg, action);
> + switch (action)
> + {
> + case F_DUPFD_CLOEXEC:
> + {
> + int target = va_arg (arg, int);
Is the arg of type 'int' or 'long'? POSIX says it's an 'int'. But
the Linux man page
<http://www.kernel.org/doc/man-pages/online/pages/man2/fcntl.2.html>
says it's 'long' "in most cases", and indeed glibc's fcntl.c implementation
uses a 'void *', that is, the same as a 'long'.
It makes a difference on big-endian 64-bit platforms (SPARC64, PPC64),
> --- a/lib/fcntl.in.h
> +++ b/lib/fcntl.in.h
> +# define F_DUPFD_CLOEXEC 0x40000000
> +# define gl_F_DUPFD_CLOEXEC 1
It would be consistent to call this macro GNULIB_defined_F_DUPFD_CLOEXEC,
for consistency with GNULIB_defined_EOVERFLOW, GNULIB_defined_SIGPIPE,
GNULIB_defined_mbstate_t and (soon) GNULIB_defined_CODESET.
> @@ -67,14 +87,10 @@ extern int open (const char *filename, int flags, ...);
> # define openat rpl_openat
> # endif
> # if !...@have_openat@ || @REPLACE_OPENAT@
> -int openat (int fd, char const *file, int flags, /* mode_t mode */ ...);
> +extern int openat (int fd, char const *file, int flags, /* mode_t mode */
> ...);
> # endif
> #elif defined GNULIB_POSIXCHECK
> -# undef openat
> -# define openat(f,u,g) \
> - (GL_LINK_WARNING ("openat is not portable - " \
> - "use gnulib module openat for portability"), \
> - openat)
> +/* Can't provide link warning without support for C99 variadic macros. */
> #endif
>
> #ifdef __cplusplus
This is mostly unrelated. IMO it belongs in a separate commit.
> --- a/m4/fcntl.m4
> +++ b/m4/fcntl.m4
> ...
> + AC_CACHE_CHECK([whether fcntl handles F_DUPFD correctly],
> + [gl_cv_func_fcntl_f_dupfd_works],
> + [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
> +#include <fcntl.h>
> +]], [[return fcntl (0, F_DUPFD, -1) != -1;
> + ]])],
> + [gl_cv_func_fcntl_f_dupfd_works=yes],
> + [gl_cv_func_fcntl_f_dupfd_works=no],
> + [gl_cv_func_fcntl_f_dupfd_works="guessing no"])])
For the sake of the people who cross-compile to a Linux/ARM or Linux/SH
platform, it would be good to be a bit more specific about the
cross-compilation guess. Require AC_CANONICAL_HOST and do
# Guess it works on glibc systems.
case "$host_os" in
*-gnu*) gl_cv_func_fcntl_f_dupfd_works="guessing yes";;
*) gl_cv_func_fcntl_f_dupfd_works="guessing no";;
esac
> +...@item
> +This function is missing on some platforms, although the replacement
> +fails with @code{EINVAL} for unimplemented actions:
> +mingw.
It's more understandable if formulated like this:
@item
This function is missing on some platforms:
mingw.
Note that the gnulib replacement fails with @code{EINVAL} for unimplemented
actions.
> --- a/lib/fcntl.c
> +++ b/lib/fcntl.c
> ...
> #if !HAVE_FCNTL
> -# error not ported to mingw yet
> +# define rpl_fcntl fcntl
> #endif
Bizarre, but probably correct.
> index d13127e..6f26071 100644
> --- a/modules/accept4
> +++ b/modules/accept4
> @@ -9,7 +9,7 @@ m4/accept4.m4
> Depends-on:
> sys_socket
> accept
> -fcntl
> +fcntl-h
> binary-io
>
> configure.ac:
This too could be a separate commit, AFAIU.
Bruno
