diff mbox

[BZ,#16907] Sync argp.h __attribute__ with gnulib.

Message ID 20140523173457.GA26513@domone.podge
State New
Headers show

Commit Message

Ondřej Bílka May 23, 2014, 5:34 p.m. UTC 
On Fri, May 23, 2014 at 07:53:11AM -0700, Paul Eggert wrote:
> Ondřej Bílka wrote:
> >CCing gnulib. Could this be backported also there or do you have
> >different solution?
> 
> We solved this problem in a different way years ago, using something
> like this:
> 
> #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR >= 7)
> # define _GL_ATTRIBUTE_FORMAT(s) __attribute__ ((__format__ s))
> #else
> # define _GL_ATTRIBUTE_FORMAT(s)
> #endif
> 
> and then using _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3)) later,
> instead of __attribute__ ((__format__ (__printf__, 2, 3))).
> Presumably glibc could do something similar.  Glibc could use a
> different name; that's all right, gnulib will just switch to the
> name that glibc uses.

Ok, here is patch that does that with unchanged name (Does somebody have
better one?).

As I looked at header differences they are mostly minor, like using
`foo' instead "foo" in comments so it migth be worthwhile to sync them
up.


        * argp/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Define.
	(argp_error, argp_failure): Use _GL_ATTRIBUTE_FORMAT.
        * argp/argp.h (__argp_fmtstream_printf): Likewise.

 extern int argp_fmtstream_putc (argp_fmtstream_t __fs, int __ch);

Comments

Aurelien Jarno June 3, 2014, 10:59 a.m. UTC | #1 
On Fri, May 23, 2014 at 07:34:57PM +0200, Ondřej Bílka wrote:
> On Fri, May 23, 2014 at 07:53:11AM -0700, Paul Eggert wrote:
> > Ondřej Bílka wrote:
> > >CCing gnulib. Could this be backported also there or do you have
> > >different solution?
> > 
> > We solved this problem in a different way years ago, using something
> > like this:
> > 
> > #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR >= 7)
> > # define _GL_ATTRIBUTE_FORMAT(s) __attribute__ ((__format__ s))
> > #else
> > # define _GL_ATTRIBUTE_FORMAT(s)
> > #endif
> > 
> > and then using _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3)) later,
> > instead of __attribute__ ((__format__ (__printf__, 2, 3))).
> > Presumably glibc could do something similar.  Glibc could use a
> > different name; that's all right, gnulib will just switch to the
> > name that glibc uses.
> 
> Ok, here is patch that does that with unchanged name (Does somebody have
> better one?).
> 
> As I looked at header differences they are mostly minor, like using
> `foo' instead "foo" in comments so it migth be worthwhile to sync them
> up.
> 
> 
>         * argp/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Define.
> 	(argp_error, argp_failure): Use _GL_ATTRIBUTE_FORMAT.
>         * argp/argp.h (__argp_fmtstream_printf): Likewise.

Thanks for working on this issue. The patch looks fine to me, and I have
just tested it, it fixes the original issue. Please also note that your
patch doesn't apply cleanly, it seems there are some issue with the
context.

> @@ -521,12 +546,12 @@
>  extern void argp_failure (const struct argp_state *__restrict __state,
>  			  int __status, int __errnum,
>  			  const char *__restrict __fmt, ...)
> -     __attribute__ ((__format__ (__printf__, 4, 5)));
> +     _GL_ATTRIBUTE_FORMAT ((__printf__, 4, 5));
>  extern void __argp_failure (const struct argp_state *__restrict __state,
>  			    7int __status, int __errnum,

Especially there with this extra "7".
diff mbox

Patch

--- argp/argp.h	2014-05-19 11:41:01.642730292 +0200
+++ ../argp/argp.h	2013-09-08 21:46:15.382388772 +0200
@@ -35,19 +34,16 @@ 
 # define __NTH(fct) fct __THROW
 #endif
 
-#ifndef __attribute__
-/* This feature is available in gcc versions 2.5 and later.  */
-# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 5) || \
-  defined __STRICT_ANSI__
-#  define __attribute__(Spec) /* empty */
-# endif
-/* The __-protected variants of `format' and `printf' attributes
-   are accepted by gcc versions 2.6.4 (effectively 2.7) and later.  */
-# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 7) || \
-  defined __STRICT_ANSI__
-#  define __format__ format
-#  define __printf__ printf
-# endif
+/* The __attribute__ feature is available in gcc versions 2.5 and later.
+   The __-protected variants of the attributes 'format' and 'printf' are
+   accepted by gcc versions 2.6.4 (effectively 2.7) and later.
+   We enable _GL_ATTRIBUTE_FORMAT only if these are supported too, because
+   gnulib and libintl do '#define printf __printf__' when they override
+   the 'printf' function.  */
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7)
+# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec))
+#else
+# define _GL_ATTRIBUTE_FORMAT(spec) /* empty */
 #endif
 
 /* GCC 2.95 and later have "__restrict"; C99 compilers have
@@ -496,19 +519,19 @@ 
 			       FILE *__restrict __stream,
 			       unsigned int __flags);
 
 /* Possibly output the standard usage message for ARGP to stderr and exit.  */
 extern void argp_usage (const struct argp_state *__state);
 extern void __argp_usage (const struct argp_state *__state);
 
 /* If appropriate, print the printf string FMT and following args, preceded
    by the program name and `:', to stderr, and followed by a `Try ... --help'
    message, then exit (1).  */
 extern void argp_error (const struct argp_state *__restrict __state,
 			const char *__restrict __fmt, ...)
-     __attribute__ ((__format__ (__printf__, 2, 3)));
+     _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3));
 extern void __argp_error (const struct argp_state *__restrict __state,
 			  const char *__restrict __fmt, ...)
-     __attribute__ ((__format__ (__printf__, 2, 3)));
+     _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3));
 
 /* Similar to the standard gnu error-reporting function error(), but will
    respect the ARGP_NO_EXIT and ARGP_NO_ERRS flags in STATE, and will print
@@ -521,12 +546,12 @@ 
 extern void argp_failure (const struct argp_state *__restrict __state,
 			  int __status, int __errnum,
 			  const char *__restrict __fmt, ...)
-     __attribute__ ((__format__ (__printf__, 4, 5)));
+     _GL_ATTRIBUTE_FORMAT ((__printf__, 4, 5));
 extern void __argp_failure (const struct argp_state *__restrict __state,
 			    7int __status, int __errnum,
 			    const char *__restrict __fmt, ...)
-     __attribute__ ((__format__ (__printf__, 4, 5)));
+     _GL_ATTRIBUTE_FORMAT ((__printf__, 4, 5));
 
 /* Returns true if the option OPT is a valid short option.  */
 extern int _option_is_short (const struct argp_option *__opt) __THROW;
 extern int __option_is_short (const struct argp_option *__opt) __THROW;
--- argp/argp-fmtstream.h	2014-05-19 11:41:01.642730292 +0200
+++ ../argp/argp-fmtstream.h	2013-09-08 21:46:15.378388647 +0200
@@ -29,22 +28,19 @@ 
 #include <string.h>
 #include <unistd.h>
 
-#ifndef __attribute__
-/* This feature is available in gcc versions 2.5 and later.  */
-# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 5) || \
-  defined __STRICT_ANSI__
-#  define __attribute__(Spec) /* empty */
-# endif
-/* The __-protected variants of `format' and `printf' attributes
-   are accepted by gcc versions 2.6.4 (effectively 2.7) and later.  */
-# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 7) || \
-  defined __STRICT_ANSI__
-#  define __format__ format
-#  define __printf__ printf
-# endif
+/* The __attribute__ feature is available in gcc versions 2.5 and later.
+   The __-protected variants of the attributes 'format' and 'printf' are
+   accepted by gcc versions 2.6.4 (effectively 2.7) and later.
+   We enable _GL_ATTRIBUTE_FORMAT only if these are supported too, because
+   gnulib and libintl do '#define printf __printf__' when they override
+   the 'printf' function.  */
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7)
+# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec))
+#else
+# define _GL_ATTRIBUTE_FORMAT(spec) /* empty */
 #endif
 
 #if defined (__GNU_LIBRARY__) && defined (HAVE_LINEWRAP_H)
 /* line_wrap_stream is available, so use that.  */
 #define ARGP_FMTSTREAM_USE_LINEWRAP
 #endif
@@ -132,9 +128,9 @@ 
 
 extern ssize_t __argp_fmtstream_printf (argp_fmtstream_t __fs,
 					const char *__fmt, ...)
-     __attribute__ ((__format__ (printf, 2, 3)));
+     _GL_ATTRIBUTE_FORMAT ((printf, 2, 3));
 extern ssize_t argp_fmtstream_printf (argp_fmtstream_t __fs,
 				      const char *__fmt, ...)
-     __attribute__ ((__format__ (printf, 2, 3)));
+     _GL_ATTRIBUTE_FORMAT ((printf, 2, 3));
 
 extern int __argp_fmtstream_putc (argp_fmtstream_t __fs, int __ch);