Re: GL_LINK_WARNING

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Bruno Haible on 12/15/2009 4:51 PM:
>>> to instead use
>>>  #define func (GL_LINK_WARNING("..."),func)
> 
> You would need that only for functions that take a variable number of 
> arguments.
> Currently
>   - openat, fprintf, printf, snprintf, fprintf are already using your proposed
>     idiom,
>   - ioctl is not, and should be changed.

openat was only half-using it.  dprintf was also broken.  I'm committing
this as a stop-gap measure, until I finish the much bigger job of the
attribute approach.

>   - The trick is to write
>       #if HAVE_DECL_FOO
>       extern __typeof__(foo) foo __attribute__ ((__warning__ ("message")));
>       #endif

Thanks for the tip.  And you are right - as long as we depend on gcc, then
we can use __typeof__ to make applying the attribute typesafe via a single
reusable macro (where the macro takes two arguments: foo and "message"),
without needing to retype the parameter list for each function thus labeled.

> I like it. You can leave the old 'link-warning' module as it is; some other
> package may be using it.

Good point.

> About the name: GL_USAGE_WARNING? GL_USE_WARNING? I'm not an English speaker,
> but "usage" reminds me the --help explanations of every program.

But in English, USE_WARNING parses ambiguously - is it the active verb
(rhyming with "lose"), or the noun (rhyming with "loose")?  With
_GL_ARG_NONNULL, we have noun-adjective; that is, the macro is telling us
that an ARG has the property of being NONNULL.  We want the macro in
question to convey the fact that using a function will trigger a warning.
 USAGE_WARNING always implies the noun.  So I'm starting to think
_GL_WARN_ON_USE may be a better name.

> About the prefix: As discussed a couple of days ago, with arg-nonnull.h, 
> better
> use the prefix _GL_ than GL_.

Agreed.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksoYkcACgkQ84KuGfSFAYDCuACgkaZuCmiMVD4/2PvmntT22t2I
DgsAn2HGmwQvx0APjNYTEgs40H0OuAt2
=w9oF
-----END PGP SIGNATURE-----

From c150f13de37b0881b46da70a6977cdce3ca67922 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Tue, 15 Dec 2009 06:51:24 -0700
Subject: [PATCH] fcntl-h, stdio, sys_ioctl: fix declarations

We cannot use a function-like macro to provide a link-time warning
of a variadic function during GNULIB_POSIXCHECK; at least, not
without assuming C99 variadic macros.  We can, however,
use an object-like macro (as was already done for printf).

On the other hand, this patch is only a stop-gap measure to fix
an obvious bug; a more complete patch that switches from a
link-time warning (specific to GNU ld, gcc, and ELF image) to
a compile-time attribute is in the works.

* lib/stdio.in.h (dprintf): Use of link warning on a variadic
function must not take arguments.
* lib/sys_ioctl.in.h (ioctl): Likewise.
* lib/fcntl.in.h (openat): Likewise.  Declare extern.
(open): Add a link warning.

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

diff --git a/ChangeLog b/ChangeLog
index 7655b07..efa426c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-12-15  Eric Blake  <address@hidden>
+
+       fcntl-h, stdio, sys_ioctl: fix declarations
+       * lib/stdio.in.h (dprintf): Use of link warning on a variadic
+       function must not take arguments.
+       * lib/sys_ioctl.in.h (ioctl): Likewise.
+       * lib/fcntl.in.h (openat): Likewise.  Declare extern.
+       (open): Add a link warning.
+
 2009-12-15  Jim Meyering  <address@hidden>

        areadlink, areadlink-with-size: relax license to LGPLv2+
@@ -5,7 +14,7 @@
        * modules/areadlink-with-size (License): Likewise.

 2009-12-15  Joel E. Denny  <address@hidden>
-            Bruno Haible  <address@hidden>
+           Bruno Haible  <address@hidden>

        *printf: Fix memory leak.
        * lib/fprintf.c (fprintf): Free memory allocated by vasnprintf.
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index 75e1b55..953e065 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -61,6 +61,12 @@ extern "C" {
 #  define open rpl_open
 extern int open (const char *filename, int flags, ...) _GL_ARG_NONNULL ((1));
 # endif
+#elif defined GNULIB_POSIXCHECK
+# undef open
+# define open \
+    (GL_LINK_WARNING ("open is not always POSIX compliant - " \
+                      "use gnulib module open for portability"), \
+     open)
 #endif

 #if @GNULIB_OPENAT@
@@ -69,12 +75,12 @@ extern int open (const char *filename, int flags, ...) 
_GL_ARG_NONNULL ((1));
 #  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) \
+# define openat \
     (GL_LINK_WARNING ("openat is not portable - " \
                       "use gnulib module openat for portability"), \
      openat)
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 0bcf154..a52f65f 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -80,10 +80,10 @@ extern int dprintf (int fd, const char *format, ...)
 # endif
 #elif defined GNULIB_POSIXCHECK
 # undef dprintf
-# define dprintf(d,f,a) \
+# define dprintf \
     (GL_LINK_WARNING ("dprintf is unportable - " \
                       "use gnulib module dprintf for portability"), \
-     dprintf (d, f, a))
+     dprintf)
 #endif

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


-- 
1.6.5.rc1