bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: fcntl for mingw


From: Bruno Haible
Subject: Re: fcntl for mingw
Date: Fri, 11 Dec 2009 16:38:17 +0100
User-agent: KMail/1.9.9

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
> address@hidden
> +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 address@hidden@ || @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

> address@hidden
> +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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]