Re: fcntl for mingw

[Eric Blake]

Bruno Haible <bruno <at> clisp.org> writes:

> >  #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.

Agreed.  So here's the first draft of a patch series I wrote for this.  But in 
the process of writing it, I noticed a few things.  There are a few other 
functions with the same bug as openat (dprintf in stdio.in.h, ioctl in 
sys_ioctl.in.h) where the GNULIB_POSIXCHECK macro is not properly variadic.  
But the stdio.in.h file already handles the variadic printf, by attaching the 
link warning to any use of the function pointer, rather than just to function-
call syntax.  For that matter, the approach used by printf is even better than 
using a variadic macro, since it will issue the link warning for (admittedly 
uncommon) code like:
 int (*func) (const char *, ...) = printf;
or
 (printf) ("%s","");
while still giving a warning in the desired typical case of
 printf ("%s","");


So, what should I do?  Options:
1. Check in the patches below as-is
2. Ditch this series, and instead go and change all uses of GNULIB_POSIXCHECK 
that used
 #define func(args) (GL_LINK_WARNING("..."),func(args))
to instead use
 #define func (GL_LINK_WARNING("..."),func)
3. Like 2, but also check in the new va-args module (others might have a use 
for it, even though it would be unused in gnulib at this point)

>From 1cc1c7e33fe03afaab4c011ebcc0a25d79190e27 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 15 Dec 2009 06:51:24 -0700
Subject: [PATCH 1/3] openat: fix declaration

* lib/fcntl.in.h (openat): Declare extern.  Avoid non-variadic
link warning.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |    8 +++++++-
 lib/fcntl.in.h |   10 ++++------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7655b07..8381498 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-12-15  Eric Blake  <address@hidden>
+
+       openat: fix declaration
+       * lib/fcntl.in.h (openat): Declare extern.  Avoid non-variadic
+       link warning.
+
 2009-12-15  Jim Meyering  <address@hidden>

        areadlink, areadlink-with-size: relax license to LGPLv2+
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index 75e1b55..f266e9b 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -61,6 +61,8 @@ extern "C" {
 #  define open rpl_open
 extern int open (const char *filename, int flags, ...) _GL_ARG_NONNULL ((1));
 # endif
+#elif defined GNULIB_POSIXCHECK
+/* Can't provide link warning without support for C99 variadic macros.  */
 #endif

 #if @GNULIB_OPENAT@
@@ -69,15 +71,11 @@ 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 */ ...)
      _GL_ARG_NONNULL ((2));
 # 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
-- 
1.6.4.2


>From b408f7b30ce8a09e76f98f5a5a121badc18aa496 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 15 Dec 2009 06:59:43 -0700
Subject: [PATCH 2/3] va-args: new module

* modules/va-args: New file.
* m4/va-args.m4 (gl_VA_ARGS): Likewise.
* MODULES.html.sh (Core language properties): Mention it.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog       |    5 +++++
 MODULES.html.sh |    1 +
 m4/va-args.m4   |   25 +++++++++++++++++++++++++
 modules/va-args |   20 ++++++++++++++++++++
 4 files changed, 51 insertions(+), 0 deletions(-)
 create mode 100644 m4/va-args.m4
 create mode 100644 modules/va-args

diff --git a/ChangeLog b/ChangeLog
index 8381498..8967134 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2009-12-15  Eric Blake  <address@hidden>

+       va-args: new module
+       * modules/va-args: New file.
+       * m4/va-args.m4 (gl_VA_ARGS): Likewise.
+       * MODULES.html.sh (Core language properties): Mention it.
+
        openat: fix declaration
        * lib/fcntl.in.h (openat): Declare extern.  Avoid non-variadic
        link warning.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 9d5664a..7f079d3 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2000,6 +2000,7 @@ func_all_modules ()
   func_module func
   func_module inline
   func_module longlong
+  func_module va-args
   func_module vararrays
   func_end_table

diff --git a/m4/va-args.m4 b/m4/va-args.m4
new file mode 100644
index 0000000..12d099b
--- /dev/null
+++ b/m4/va-args.m4
@@ -0,0 +1,25 @@
+# va-args.m4 serial 1
+dnl Copyright (C) 2009 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl Test for variadic macro support.
+dnl Define HAVE_VA_ARGS if macros can use __VA_ARGS__.
+AC_DEFUN([gl_VA_ARGS],
+[
+  AC_CACHE_CHECK([whether the preprocessor allows variadic macros],
+    [gl_cv_c_va_args],
+    [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+       #include <stdio.h>
+       #define a(func, arg, ...) func (arg, __VA_ARGS__)
+       #define b(...) a (__VA_ARGS__)
+      ]], [[b (printf, "%s", "test");]])],
+      [gl_cv_c_va_args=yes], [gl_cv_c_va_args=no])
+    ])
+  if test $gl_cv_c_va_args = yes; then
+    AC_DEFINE([HAVE_VA_ARGS], [1],
+      [Define to 1 if the preprocessor supports variadic macros and
+       __VA_ARGS__.])
+  fi
+])
diff --git a/modules/va-args b/modules/va-args
new file mode 100644
index 0000000..79fb163
--- /dev/null
+++ b/modules/va-args
@@ -0,0 +1,20 @@
+Description:
+Detect whether the preprocessor supports C99 variadic macros and __VA_ARGS__.
+
+Files:
+m4/va-args.m4
+
+Depends-on:
+
+configure.ac:
+gl_VA_ARGS
+
+Makefile.am:
+
+Include:
+
+License:
+LGPL
+
+Maintainer:
+Eric Blake
-- 
1.6.4.2


>From ad40c426b3e6de3888158393daac56ee5feabe9b Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 15 Dec 2009 08:28:57 -0700
Subject: [PATCH 3/3] va-args: use it to improve GNULIB_POSIXCHECK

Provide a variadic macro when using -DGNULIB_POSIXCHECK to warn
about use of a variadic function provided in gnulib but not used
by the application.  No need to substitute something like
@HAVE_VA_ARGS@ in Makefile.am, since the use of GNULIB_POSIXCHECK
is only supported via manual use; this does, however, mean that
users must consistently have <config.h> included before system
headers if they plan on using GNULIB_POSIXCHECK.  The only time
that GL_LINK_WARNING will do anything is with gcc, where
HAVE_VA_ARGS will be defined, but GNULIB_POSIXCHECK must still
gracefully work with non-gcc compilers.

* modules/fcntl-h (Depends-on): Add va-args.
* modules/stdio (Depends-on): Likewise.
* modules/sys_ioctl (Depends-on): Likewise.
* lib/fcntl.in.h (open, openat): Use variadic link warning.
* lib/stdio.in.h (dprintf, fprintf, printf, snprintf, sprintf):
Likewise.
* lib/sys_ioctl.in.h (ioctl): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog          |    9 +++++++++
 lib/fcntl.in.h     |   17 +++++++++++++----
 lib/stdio.in.h     |   30 +++++++++++++++---------------
 lib/sys_ioctl.in.h |    6 +++---
 modules/fcntl-h    |    5 +++--
 modules/stdio      |    3 ++-
 modules/sys_ioctl  |    1 +
 7 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8967134..a318ad5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2009-12-15  Eric Blake  <address@hidden>

+       va-args: use it to improve GNULIB_POSIXCHECK
+       * modules/fcntl-h (Depends-on): Add va-args.
+       * modules/stdio (Depends-on): Likewise.
+       * modules/sys_ioctl (Depends-on): Likewise.
+       * lib/fcntl.in.h (open, openat): Use variadic link warning.
+       * lib/stdio.in.h (dprintf, fprintf, printf, snprintf, sprintf):
+       Likewise.
+       * lib/sys_ioctl.in.h (ioctl): Likewise.
+
        va-args: new module
        * modules/va-args: New file.
        * m4/va-args.m4 (gl_VA_ARGS): Likewise.
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index f266e9b..35ff5d0 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -61,8 +61,13 @@ extern "C" {
 #  define open rpl_open
 extern int open (const char *filename, int flags, ...) _GL_ARG_NONNULL ((1));
 # endif
-#elif defined GNULIB_POSIXCHECK
-/* Can't provide link warning without support for C99 variadic macros.  */
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
+# undef open
+# define open(f,...)                                          \
+    (GL_LINK_WARNING ("open is not always POSIX compliant - " \
+                      "use gnulib module open for portable " \
+                      "POSIX compliance"), \
+     open (f, __VA_ARGS__))
 #endif

 #if @GNULIB_OPENAT@
@@ -74,8 +79,12 @@ extern int open (const char *filename, int flags, ...) 
_GL_ARG_NONNULL ((1));
 extern int openat (int fd, char const *file, int flags, /* mode_t mode */ ...)
      _GL_ARG_NONNULL ((2));
 # endif
-#elif defined GNULIB_POSIXCHECK
-/* Can't provide link warning without support for C99 variadic macros.  */
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
+# undef openat
+# define openat(f,...)                                          \
+    (GL_LINK_WARNING ("openat is not always portable - " \
+                      "use gnulib module openat for portability "), \
+     openat (f, __VA_ARGS__))
 #endif

 #ifdef __cplusplus
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 0bcf154..0d56dcf 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -78,12 +78,12 @@ extern "C" {
 extern int dprintf (int fd, const char *format, ...)
        __attribute__ ((__format__ (__printf__, 2, 3))) _GL_ARG_NONNULL ((2));
 # endif
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef dprintf
-# define dprintf(d,f,a) \
+# define dprintf(d,...) \
     (GL_LINK_WARNING ("dprintf is unportable - " \
                       "use gnulib module dprintf for portability"), \
-     dprintf (d, f, a))
+     dprintf (d, __VA_ARGS__))
 #endif

 #if @GNULIB_FCLOSE@
@@ -148,13 +148,13 @@ extern int fprintf (FILE *fp, const char *format, ...)
 extern int fprintf (FILE *fp, const char *format, ...)
        __attribute__ ((__format__ (__printf__, 2, 3)))
        _GL_ARG_NONNULL ((1, 2));
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef fprintf
-# define fprintf \
+# define fprintf(f,...)                                          \
     (GL_LINK_WARNING ("fprintf is not always POSIX compliant - " \
                       "use gnulib module fprintf-posix for portable " \
                       "POSIX compliance"), \
-     fprintf)
+     fprintf (f, __VA_ARGS__))
 #endif

 #if @GNULIB_FPURGE@
@@ -406,13 +406,13 @@ extern int printf (const char *format, ...)
 # define printf __printf__
 extern int printf (const char *format, ...)
        __attribute__ ((__format__ (__printf__, 1, 2))) _GL_ARG_NONNULL ((1));
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef printf
-# define printf \
+# define printf(...)                                            \
     (GL_LINK_WARNING ("printf is not always POSIX compliant - " \
                       "use gnulib module printf-posix for portable " \
                       "POSIX compliance"), \
-     printf)
+     printf (__VA_ARGS__))
 /* Don't break __attribute__((format(printf,M,N))).  */
 # define format(kind,m,n) format (__##kind##__, m, n)
 # define __format__(kind,m,n) __format__ (__##kind##__, m, n)
@@ -495,12 +495,12 @@ extern int snprintf (char *str, size_t size, const char 
*format, ...)
        __attribute__ ((__format__ (__printf__, 3, 4)))
        _GL_ARG_NONNULL ((3));
 # endif
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef snprintf
-# define snprintf \
+# define snprintf(s,...)                          \
     (GL_LINK_WARNING ("snprintf is unportable - " \
                       "use gnulib module snprintf for portability"), \
-     snprintf)
+     snprintf (s, __VA_ARGS__))
 #endif

 #if @GNULIB_SPRINTF_POSIX@
@@ -510,13 +510,13 @@ extern int sprintf (char *str, const char *format, ...)
        __attribute__ ((__format__ (__printf__, 2, 3)))
        _GL_ARG_NONNULL ((1, 2));
 # endif
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef sprintf
-# define sprintf \
+# define sprintf(s,...)                                          \
     (GL_LINK_WARNING ("sprintf is not always POSIX compliant - " \
                       "use gnulib module sprintf-posix for portable " \
                       "POSIX compliance"), \
-     sprintf)
+     sprintf (s, __VA_ARGS__))
 #endif

 #if @GNULIB_VASPRINTF@
diff --git a/lib/sys_ioctl.in.h b/lib/sys_ioctl.in.h
index 55d3b35..a7ab95a 100644
--- a/lib/sys_ioctl.in.h
+++ b/lib/sys_ioctl.in.h
@@ -52,12 +52,12 @@ extern int ioctl (int fd, int request, ... /* {void *,char 
*} arg */);
 #elif @SYS_IOCTL_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS@
 # undef ioctl
 # define ioctl ioctl_used_without_requesting_gnulib_module_ioctl
-#elif defined GNULIB_POSIXCHECK
+#elif defined GNULIB_POSIXCHECK && HAVE_VA_ARGS
 # undef ioctl
-# define ioctl(f,c,a) \
+# define ioctl(f,...) \
     (GL_LINK_WARNING ("ioctl does not portably work on sockets - " \
                       "use gnulib module ioctl for portability"), \
-     ioctl (f, c, a))
+     ioctl (f, __VA_ARGS__))
 #endif


diff --git a/modules/fcntl-h b/modules/fcntl-h
index 6eaec52..1882502 100644
--- a/modules/fcntl-h
+++ b/modules/fcntl-h
@@ -6,11 +6,12 @@ lib/fcntl.in.h
 m4/fcntl_h.m4

 Depends-on:
+arg-nonnull
+extensions
 include_next
 link-warning
-arg-nonnull
 unistd
-extensions
+va-args

 configure.ac:
 gl_FCNTL_H
diff --git a/modules/stdio b/modules/stdio
index eca3581..f8ec72f 100644
--- a/modules/stdio
+++ b/modules/stdio
@@ -7,11 +7,12 @@ lib/stdio-write.c
 m4/stdio_h.m4

 Depends-on:
+arg-nonnull
 include_next
 link-warning
-arg-nonnull
 raise
 stddef
+va-args

 configure.ac:
 gl_STDIO_H
diff --git a/modules/sys_ioctl b/modules/sys_ioctl
index 5ae8c84..0f8e229 100644
--- a/modules/sys_ioctl
+++ b/modules/sys_ioctl
@@ -9,6 +9,7 @@ Depends-on:
 include_next
 link-warning
 unistd
+va-args

 configure.ac:
 gl_SYS_IOCTL_H
-- 
1.6.4.2