Re: [Qemu-devel] [PATCH 2/2] Use new gcc format attribute gnu_printf

[Blue Swirl]

On Mon, Sep 13, 2010 at 8:02 PM, Stefan Weil <address@hidden> wrote:
> Since version 4.4.x, gcc supports additional format attributes.
>    __attribute__ ((format (gnu_printf, 1, 2)))
> should be used instead of
>    __attribute__ ((format (printf, 1, 2))
> because QEMU always uses standard format strings (even with mingw32).
>
> The patch replaces format attribute printf / __printf__ by gnu_printf.
>
> It also removes an #ifdef __GNUC__ (not needed as long as we compile
> with gcc, and for non-gcc compilers we need more changes than this).
>
> The gcc documentation uses format (not __format__), the majority in
> QEMU uses this shorter form, too. Therefore the patch also replaces
> __format__ by format.
>
> Spacing was unified (again as in the gcc documentation).

Please avoid GNU style spacing. This is C, not LISP.

> Signed-off-by: Stefan Weil <address@hidden>
> ---
>  audio/audio.h      |    5 +----
>  audio/audio_int.h  |    4 ++--
>  bsd-user/qemu.h    |    2 +-
>  cpu-all.h          |    2 +-
>  darwin-user/qemu.h |    2 +-
>  hw/xen_backend.h   |    2 +-
>  linux-user/qemu.h  |    2 +-
>  monitor.h          |    2 +-
>  qemu-common.h      |    2 +-
>  qemu-error.h       |    8 +++++---
>  qerror.h           |    2 +-
>  qjson.h            |    2 +-
>  12 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/audio/audio.h b/audio/audio.h
> index 454ade2..4439b82 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -88,10 +88,7 @@ typedef struct QEMUAudioTimeStamp {
>
>  void AUD_vlog (const char *cap, const char *fmt, va_list ap);
>  void AUD_log (const char *cap, const char *fmt, ...)
> -#ifdef __GNUC__
> -    __attribute__ ((__format__ (__printf__, 2, 3)))
> -#endif
> -    ;
> +    __attribute__ ((format (gnu_printf, 2, 3)));
>
>  void AUD_help (void);
>  void AUD_register_card (const char *name, QEMUSoundCard *card);
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 06e313f..f6a77ad 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -237,8 +237,8 @@ static inline int audio_ring_dist (int dst, int src, int 
> len)
>  }
>
>  #if defined __GNUC__
> -#define GCC_ATTR __attribute__ ((__unused__, __format__ (__printf__, 1, 2)))
> -#define GCC_FMT_ATTR(n, m) __attribute__ ((__format__ (__printf__, n, m)))
> +#define GCC_ATTR __attribute__ ((__unused__, format (gnu_printf, 1, 2)))
> +#define GCC_FMT_ATTR(n, m) __attribute__ ((format (gnu_printf, n, m)))

I'd rather move this macro to a central header and converting instead
users of other syntax. The macro could be defined differently
depending on whether gnu_printf is supported or not. Then we would not
need any -Dgnu_printf=printf in 1/2, which can cause problems.