Re: [PATCH 07/27] Replace GCC_FMT_ATTR with G_GNUC_PRINTF

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Wed, Mar 16, 2022 at 01:52:48PM +0400, marcandre.lureau@xxxxxxxxxx wrote:
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 3baa5e3790f7..f2bd050e3b9a 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -79,19 +79,12 @@
>  #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \
>                                     sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)))
>  
> -#if defined(__clang__)
> -/* clang doesn't support gnu_printf, so use printf. */
> -# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> -#else
> -/* Use gnu_printf (qemu uses standard format strings). */
> -# define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> -# if defined(_WIN32)
> +#if !defined(__clang__) && defined(_WIN32)
>  /*
>   * Map __printf__ to __gnu_printf__ because we want standard format strings 
> even
>   * when MinGW or GLib include files use __printf__.
>   */
> -#  define __printf__ __gnu_printf__
> -# endif
> +# define __printf__ __gnu_printf__
>  #endif

I'm not convinced we shold have this remaining define, even
before your patch.

For code we've implemented, we should have used __gnu_printf__
already if we know it uses GNU format on Windows.

For code in GLib, its header file uses __gnu_printf__ for anything
that relies on its portable printf replacement, which is basically
everything in GLib.

For anything else we should honour whatever they declare, and not
assume their impl is the GNU one.


I guess it is easy enough to validate by deleting this and seeing
if we get any warnings from the mingw CI jobs about printf/gnu_printf
mismatch.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|