[Spice-devel] [spice-common v3] log: Let gcc know about the logging macros which abort

[Frediano Ziglio]

> 
> This commit adds a SPICE_UNREACHABLE macro (courtesy of Frediano)
> so that gcc does not think that code control can go past
> spice_return_{val_,}if_fail(), spice_critical() and spice_error()
> 
> This avoids this kind of warnings:
> 
> fallthrough.c:
> 
>  #include "log.h"
> 
> int main(int argc, char **argv)
> {
>     switch(argc) {
>         case 1:
>             spice_critical("foo");
>        default:
>             return 0;
>     }
> }
> 
> $ gcc  -c    $(pkg-config --cflags --libs glib-2.0 spice-protocol)
>        -I common   -Wimplicit-fallthrough=5 ./fallthrough.c
> In file included from ./fallthrough.c:1:
> ./fallthrough.c: In function 'main':
> common/log.h:73:5: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
>    73 |     spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, ""
>    format, ## __VA_ARGS__); \
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./fallthrough.c:8:25: note: in expansion of macro 'spice_critical'
>     8 |                         spice_critical("foo");
>       |                         ^~~~~~~~~~~~~~
> ./fallthrough.c:9:17: note: here
>     9 |                 default:
>       |                 ^~~~~~~
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
> Maybe a bit too much for a single commit, I can split :)
> 

Acked-by: Frediano Ziglio <fziglio at redhat.com>

>  common/log.h    | 8 ++++++--
>  common/macros.h | 8 ++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/common/log.h b/common/log.h
> index 7c67e7a..201c87a 100644
> --- a/common/log.h
> +++ b/common/log.h
> @@ -39,16 +39,18 @@ void spice_log(GLogLevelFlags log_level,
>                 const char *format,
>                 ...) G_GNUC_PRINTF(4, 5);
>  
> +/* FIXME: name is misleading, this aborts.. */
>  #define spice_return_if_fail(x) G_STMT_START {                          \
>      if G_LIKELY(x) { } else {                                           \
> -        spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition
> `%s' failed", #x); \
> +        spice_critical("condition `%s' failed", #x);                    \
>          return;                                                         \
>      }                                                                   \
>  } G_STMT_END
>  
> +/* FIXME: name is misleading, this aborts.. */
>  #define spice_return_val_if_fail(x, val) G_STMT_START {                 \
>      if G_LIKELY(x) { } else {                                           \
> -        spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__,
> "condition `%s' failed", #x); \
> +        spice_critical("condition `%s' failed", #x);                    \
>          return (val);                                                   \
>      }                                                                   \
>  } G_STMT_END
> @@ -71,10 +73,12 @@ void spice_log(GLogLevelFlags log_level,
>  
>  #define spice_critical(format, ...) G_STMT_START {
>  \
>      spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format,
>      ## __VA_ARGS__); \
> +    SPICE_UNREACHABLE;
> \
>  } G_STMT_END
>  
>  #define spice_error(format, ...) G_STMT_START {                         \
>      spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, "" format, ##
>      __VA_ARGS__); \
> +    SPICE_UNREACHABLE;
> \
>  } G_STMT_END
>  
>  #define spice_warn_if_fail(x) G_STMT_START {            \
> diff --git a/common/macros.h b/common/macros.h
> index 2f24ada..92cd82c 100644
> --- a/common/macros.h
> +++ b/common/macros.h
> @@ -55,4 +55,12 @@
>  
>  #define SPICE_VERIFY(cond) verify_expr(cond, (void)1)
>  
> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
> +#define SPICE_UNREACHABLE __builtin_unreachable()
> +#elif defined(_MSC_VER)
> +#define SPICE_UNREACHABLE __assume(0)
> +#else
> +#define SPICE_UNREACHABLE for(;;) continue
> +#endif
> +
>  #endif // H_SPICE_COMMON_MACROS

Frediano