[Qemu-devel] Handling of fall through code (was: [PATCH v8 04/87] target/mips: Mark switch fallthroughs with interpretable comments

[Stefan Weil]

Am 13.08.18 um 19:52 schrieb Aleksandar Markovic:

> From: Aleksandar Markovic <address@hidden>
>
> 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