[libvirt] [PATCH] Fix build with GCC 8 new warnings

Jiri Denemark jdenemar at redhat.com
Tue Feb 13 12:58:44 UTC 2018


On Tue, Feb 13, 2018 at 12:21:11 +0000, Daniel P. Berrangé wrote:
> GCC 8 added a -Wcast-function-type warning to -Wextra by
> default. This complains if you try to explicitly cast
> one function signature to another function signature
> with incompatible args. This is something we do many
> times in libvirt especially with event callbacks. The
> hack to silence the warning requires casting via a
> void(*)(void) signature before casting to the desired
> type. This gross, so we just disable this new warning.
> 
> GCC 8 also became more fussy about detecting switch
> fallthroughs. First it doesn't like it if you have
> a fallthrough attribute that is not before a case
> statement. e.g.
> 
>    FOO:
>    BAR:
>    WIZZ:
>       ATTRIBUTE_FALLTHROUGH;
> 
> Is unacceptable as there's no final case statement,
> so while FOO & BAR are falling through, WIZZ is
> not falling through. IOW, GCC wants us to write
> 
>   FOO:
>   BAR:
>     ATTRIBUTE_FALLTHROUGH;
>   WIZZ:
> 
> Second, it will report risk of fallthrough even if you
> have a case statement for every single enum value, but
> only if the switch is nested inside another switch and
> the outer case statement has no final break. This is
> is arguably valid because despite the fact that we have
> cast from "int" to the enum typedef, nothing guarantees
> that the variable we're switching on only contains values
> that have corresponding switch labels. e.g.
> 
>    int domstate = 87539319;
>    switch ((virDomainState)domstate) {
>       ...
>    }
> 
> will not match enum value, but also not raise any kind
> of compiler warning. So it is right to complain about
> risk of fallthrough if no default: is present.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  m4/virt-compile-warnings.m4    |  2 ++
>  src/qemu/qemu_domain.c         | 14 +++++++-------
>  src/qemu/qemu_domain_address.c | 11 +++++++++++
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> index b9c974842..4b35a6f6b 100644
> --- a/m4/virt-compile-warnings.m4
> +++ b/m4/virt-compile-warnings.m4
> @@ -177,6 +177,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>      # with gl_MANYWARN_COMPLEMENT
>      # So we have -W enabled, and then have to explicitly turn off...
>      wantwarn="$wantwarn -Wno-sign-compare"
> +    # We do "bad" function casts all the time for event callbacks
> +    wantwarn="$wantwarn -Wno-cast-function-type"
>  
>      # GNULIB expects this to be part of -Wc++-compat, but we turn
>      # that one off, so we need to manually enable this again
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 178ec24ae..560436f8e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -175,9 +175,9 @@ qemuDomainAsyncJobPhaseToString(qemuDomainAsyncJob job,
>      case QEMU_ASYNC_JOB_NONE:
>      case QEMU_ASYNC_JOB_LAST:
>          ATTRIBUTE_FALLTHROUGH;
> +    default:
> +        return "none";
>      }
> -
> -    return "none";
>  }
>  
>  int

Please don't do this. I hope there's a better way of silencing the
warnings. The reason for not providing a default value is to make sure
gcc emits a warning one the corresponding enum is changed. This change
will prevent gcc from reporting all places where the new item needs to
be explicitly handled.

Jirka




More information about the libvir-list mailing list