Re: [PATCH] Fixes: Fallthrough warning on line 270 of qemu/qapi/opts-visitor.c

[Philippe Mathieu-Daudé]

Hi Rohit,

Congratulation for your first patch! It is in very
good shape already :)

It is easier for the reviewers if you start the patch subject with
the name of the subsystem concerned, or the file modified:

"qapi/opts-visitor: Add missing fallthrough annotations"

On 8/15/20 3:00 PM, Rohit Shinde wrote:
> Added the fallthrough comment so that the compiler doesn't emit an error on 
> compiling with the -Wimplicit-fallthrough flag.

If possible align the description to 72 chars.

> 
> Signed off by: Rohit Shinde

The tag is written "Signed-off-by" with '-', then your "name <email>":

Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com>

If you configure your git client, using 'git-commit -s' will
automatically add the S-o-b tag:

$ git config user.name "Rohit Shinde"
$ git config user.email "rohit.shinde12194@gmail.com"
$ git commit -s

> ---
>  qapi/opts-visitor.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 7781c23a42..43cf60d3a0 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -266,6 +266,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
>          }
>          ov->list_mode = LM_IN_PROGRESS;
>          /* range has been completed, fall through in order to pop option */
> +        __attribute__((fallthrough));

C uses attributes when declaring a type/variable/function.
Here this is inside a function body, not a declaration.
A simple "/* fallthrough */" comment will make the compiler happy.
You can see a similar patch for example:
https://git.qemu.org/?p=qemu.git;a=blobdiff;f=disas/sh4.c;h=dcdbdf26d8;hp=55ef865a3;hb=ccb237090f;hpb=7aa12aa215

When you find an issue that might have already been fixed elsewhere
in the repository, 'git-log -p' is your friend. Since the commits are
patches already accepted/merged, they might be a good source to learn
(how the issue was fixed, how the bug was described, ...).

Regards,

Phil.

>  
>      case LM_IN_PROGRESS: {
>          const QemuOpt *opt;
>