Re: [libvirt] [PATCH 4/4] Add ATTRIBUTE_FALLTHROUGH for switch cases without break
On Wed, Feb 22, 2017 at 02:49:12PM -0600, Eric Blake wrote:
On 02/22/2017 11:52 AM, Daniel P. Berrange wrote:
> In GCC 7 there is a new warning triggered when a switch
> case has a conditional statement (eg if ... else...) and
> some of the code paths fallthrough to the next switch
> statement. e.g.
>
> conf/domain_conf.c: In function 'virDomainChrEquals':
> conf/domain_conf.c:14926:12: error: this statement may fall through
[-Werror=implicit-fallthrough=]
> if (src->targetTypeAttr != tgt->targetTypeAttr)
> ^
> +++ b/src/conf/domain_conf.c
> @@ -14925,7 +14925,12 @@ virDomainChrEquals(virDomainChrDefPtr src,
> case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
> if (src->targetTypeAttr != tgt->targetTypeAttr)
> return false;
> +
> + ATTRIBUTE_FALLTHROUGH;
> +
I understand this one...
> case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> + ATTRIBUTE_FALLTHROUGH;
> +
> case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
...but was this one necessary, or is gcc smart enough to know that two
consecutive labels never needs an explicit fallthrough?
Yeah, that second one was bogus - i added it by mistake.
Also, is it sufficient to spell it:
/* fall through */
so that Coverity can also recognize it? Or does gcc not recognize the
magic comment?
IIUC, it only likes an all lowercase 'fallthrough' comment, but I'm
not inclined to use that. CLang doesn't parse comments at all and
relies on the attribute annotation.
Given that both GCC & CLang now support the same attribute name for
this, I would expect Coverity will learn about it too in the not too
distant future.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|