Am 13.08.18 um 19:52 schrieb Aleksandar Markovic:
From: Aleksandar Markovic <amarko...@wavecomp.com>
Mark switch fallthroughs with comments, in cases fallthroughs
are intentional.
This is a general problem all over the QEMU code. I usually compile with
nearly all warnings enabled and get now lots of errors with the latest
code and after updating to gcc-8.3.0 (Debian buster). It should be
reproducible by enabling -Werror=implicit-fallthrough.
The current situation is like this:
- Some code has fallthrough comments which are accepted by the compiler.
- Other code has fallthrough comments which are not accepted (resulting
in a compiler error).
- Some code is correct, but has no indication that the fallthrough is
intentional.
- There is also fallthrough code which is obviously not correct (even in
target/mips/translate.c).
I suggest to enable -Werror=implicit-fallthrough by default and add a
new macro to mark all fallthrough locations which are correct, but not
accepted by the compiler.
This can be done with a definition for GCC compatible compilers in
include/qemu/compiler.h:
#define QEMU_FALLTHROUGH __attribute__ ((fallthrough))
Then fallthrough code would look like this:
case 1:
do_something();
QEMU_FALLTHROUGH;
case 2:
VIXL_FALLTHROUGH also needs a similar definition to work with gcc-8.3.0.
Please comment. Would you prefer another macro name or a macro with
parentheses like this:
#define QEMU_FALLTHROUGH() __attribute__ ((fallthrough))
As soon as there is consensus on the macro name and form, I can send a
patch which adds it (but would not mind if someone else adds it).
Then I suggest that the maintainers build with the fallthrough warning
enabled and fix all warnings, either by using the new macro or by adding
the missing break.
Finally we can enforce the warning by default.
Another macro which is currently missing is a scanf variant of GCC_FMT_ATTR.
I suggest to add and use a GCC_SCANF_ATTR macro:
#define GCC_SCANF_ATTR(n, m) __attribute__((format(gnu_scanf, n, m)))
A more regular solution would require renaming GCC_FMT_ATTR to
GCC_FMT_PRINTF and use GCC_FMT_SCANF for the new macro.
Regards
Stefan Weil
