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)
^
conf/domain_conf.c:14928:5: note: here
case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
^~~~
conf/domain_conf.c: In function 'virDomainChrDefFormat':
conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (def->targetTypeAttr) {
^
conf/domain_conf.c:22151:5: note: here
default:
^~~~~~~
GCC introduced a __attribute__((fallthrough)) to let you
indicate that this is intentionale behaviour rather than
a bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
src/conf/network_conf.c | 3 ++-
src/internal.h | 8 ++++++++
src/lxc/lxc_container.c | 2 +-
src/network/bridge_driver.c | 6 ++++++
tools/virsh-edit.c | 2 +-
6 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f718b9a..17995f7 100644
--- a/src/conf/domain_conf.c
+++ 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;
+
case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+ ATTRIBUTE_FALLTHROUGH;
+
case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
return src->target.port == tgt->target.port;
break;
@@ -22148,6 +22153,8 @@ virDomainChrDefFormat(virBufferPtr buf,
def->target.port);
break;
}
+ ATTRIBUTE_FALLTHROUGH;
+
default:
virBufferAsprintf(buf, "<target port='%d'/>\n",
def->target.port);
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0e20dac..48e0001 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2442,7 +2442,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->name);
goto error;
}
- /* fall through to next case */
+ ATTRIBUTE_FALLTHROUGH;
+
case VIR_NETWORK_FORWARD_BRIDGE:
if (def->delay || stp) {
virReportError(VIR_ERR_XML_ERROR,
diff --git a/src/internal.h b/src/internal.h
index 334659d..74a43fa 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -218,6 +218,10 @@
# endif
# endif
+# ifndef ATTRIBUTE_FALLTHROUGH
+# define ATTRIBUTE_FALLTHROUGH __attribute__((fallthrough))
+# endif
+
# else
# ifndef ATTRIBUTE_UNUSED
# define ATTRIBUTE_UNUSED
@@ -228,6 +232,10 @@
# ifndef ATTRIBUTE_RETURN_CHECK
# define ATTRIBUTE_RETURN_CHECK
# endif
+#
+# ifndef ATTRIBUTE_FALLTHROUGH
+# define ATTRIBUTE_FALLTHROUGH do {} while(0)
+# endif
# endif /* __GNUC__ */
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 601b9b0..99bd7e9 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2042,7 +2042,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
default: /* User specified capabilities to drop */
toDrop = (state == VIR_TRISTATE_SWITCH_OFF);
}
- /* Fallthrough */
+ ATTRIBUTE_FALLTHROUGH;
case VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW:
if (policy == VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 06759c6..c5ec282 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2715,6 +2715,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
* VIR_NETWORK_FORWARD_BRIDGE with no bridge device defined
* (since that is macvtap bridge mode).
*/
+ ATTRIBUTE_FALLTHROUGH;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
@@ -2792,6 +2794,8 @@ networkShutdownNetwork(virNetworkDriverStatePtr driver,
* VIR_NETWORK_FORWARD_BRIDGE with no bridge device defined
* (since that is macvtap bridge mode).
*/
+ ATTRIBUTE_FALLTHROUGH;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
@@ -4974,6 +4978,8 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
* fall through if netdef->bridge wasn't set, since that is
* macvtap bridge mode network.
*/
+ ATTRIBUTE_FALLTHROUGH;
+
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c
index 16d6705..92a00b7 100644
--- a/tools/virsh-edit.c
+++ b/tools/virsh-edit.c
@@ -140,7 +140,7 @@ do {
goto redefine;
break;
}
- /* fall-through */
+ ATTRIBUTE_FALLTHROUGH;
#endif
default:
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Feb 22, 2017 at 05:52:05PM +0000, 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)
> ^
> conf/domain_conf.c:14928:5: note: here
> case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> ^~~~
> conf/domain_conf.c: In function 'virDomainChrDefFormat':
> conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
> if (def->targetTypeAttr) {
> ^
> conf/domain_conf.c:22151:5: note: here
> default:
> ^~~~~~~
>
> GCC introduced a __attribute__((fallthrough)) to let you
> indicate that this is intentionale behaviour rather than
> a bug.
BTW, CLang has apparently had an -Wimplicit-fallthrough
warning flag for a while, and also seems to have the same
__attribute__((fallthrough)), but I've been unable to get
CLang to trigger such warnings, and hence did not enable
__attribute__((fallthrough)) on CLang. If someone can
figure out how to reproduce the warnings on clang we
could extend internal.h to surpress them for whichever
clang version introduced the warnings. As is, the
ATTRIBUTE_FALLTHROUGH turns into a no-op for CLang unless
it claims GCC 7.0 compatibility.
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/ :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Daniel P. Berrange wrote:
> On Wed, Feb 22, 2017 at 05:52:05PM +0000, 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)
> > ^
> > conf/domain_conf.c:14928:5: note: here
> > case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> > ^~~~
> > conf/domain_conf.c: In function 'virDomainChrDefFormat':
> > conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
> > if (def->targetTypeAttr) {
> > ^
> > conf/domain_conf.c:22151:5: note: here
> > default:
> > ^~~~~~~
> >
> > GCC introduced a __attribute__((fallthrough)) to let you
> > indicate that this is intentionale behaviour rather than
> > a bug.
>
>
> BTW, CLang has apparently had an -Wimplicit-fallthrough
> warning flag for a while, and also seems to have the same
> __attribute__((fallthrough)), but I've been unable to get
> CLang to trigger such warnings, and hence did not enable
> __attribute__((fallthrough)) on CLang. If someone can
> figure out how to reproduce the warnings on clang we
> could extend internal.h to surpress them for whichever
> clang version introduced the warnings. As is, the
> ATTRIBUTE_FALLTHROUGH turns into a no-op for CLang unless
> it claims GCC 7.0 compatibility.
It *looks* like -Wimplicit-fallthrough in clang is only effective for
C++11:
http://clang-developers.42468.n3.nabble.com/should-Wimplicit-fallthrough-require-C-11-td4028144.html
It mentions this commit:
http://llvm.org/viewvc/llvm-project?revision=167655&view=revision
And it seems it wasn't changed since than.
Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Feb 23, 2017 at 09:07:59PM +0400, Roman Bogorodskiy wrote:
> Daniel P. Berrange wrote:
>
> > On Wed, Feb 22, 2017 at 05:52:05PM +0000, 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)
> > > ^
> > > conf/domain_conf.c:14928:5: note: here
> > > case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> > > ^~~~
> > > conf/domain_conf.c: In function 'virDomainChrDefFormat':
> > > conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
> > > if (def->targetTypeAttr) {
> > > ^
> > > conf/domain_conf.c:22151:5: note: here
> > > default:
> > > ^~~~~~~
> > >
> > > GCC introduced a __attribute__((fallthrough)) to let you
> > > indicate that this is intentionale behaviour rather than
> > > a bug.
> >
> >
> > BTW, CLang has apparently had an -Wimplicit-fallthrough
> > warning flag for a while, and also seems to have the same
> > __attribute__((fallthrough)), but I've been unable to get
> > CLang to trigger such warnings, and hence did not enable
> > __attribute__((fallthrough)) on CLang. If someone can
> > figure out how to reproduce the warnings on clang we
> > could extend internal.h to surpress them for whichever
> > clang version introduced the warnings. As is, the
> > ATTRIBUTE_FALLTHROUGH turns into a no-op for CLang unless
> > it claims GCC 7.0 compatibility.
>
> It *looks* like -Wimplicit-fallthrough in clang is only effective for
> C++11:
>
> http://clang-developers.42468.n3.nabble.com/should-Wimplicit-fallthrough-require-C-11-td4028144.html
>
> It mentions this commit:
>
> http://llvm.org/viewvc/llvm-project?revision=167655&view=revision
>
> And it seems it wasn't changed since than.
Ah I see. So they disabled it for C, because their [[clang::fallthrough]]
magic annotation would not work under C. Hopefully they'll enable it one
day with the more normal __attribute__((fallthrough)) syntax for C.
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/ :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/22/2017 12:52 PM, 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)
> ^
> conf/domain_conf.c:14928:5: note: here
> case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> ^~~~
> conf/domain_conf.c: In function 'virDomainChrDefFormat':
> conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
> if (def->targetTypeAttr) {
> ^
> conf/domain_conf.c:22151:5: note: here
> default:
> ^~~~~~~
>
> GCC introduced a __attribute__((fallthrough)) to let you
> indicate that this is intentionale behaviour rather than
> a bug.
ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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?
Also, is it sufficient to spell it:
/* fall through */
so that Coverity can also recognize it? Or does gcc not recognize the
magic comment?
> +++ b/src/internal.h
> @@ -218,6 +218,10 @@
> # endif
> # endif
>
> +# ifndef ATTRIBUTE_FALLTHROUGH
> +# define ATTRIBUTE_FALLTHROUGH __attribute__((fallthrough))
> +# endif
If the magic /* fall through */ comment is sufficient, then we don't
need this macro.
> +++ b/src/lxc/lxc_container.c
> @@ -2042,7 +2042,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
> default: /* User specified capabilities to drop */
> toDrop = (state == VIR_TRISTATE_SWITCH_OFF);
> }
> - /* Fallthrough */
> + ATTRIBUTE_FALLTHROUGH;
Hmm - this argues at least one comment spelling that gcc does not recognize.
> +++ b/tools/virsh-edit.c
> @@ -140,7 +140,7 @@ do {
> goto redefine;
> break;
> }
> - /* fall-through */
> + ATTRIBUTE_FALLTHROUGH;
> #endif
and another.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.