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

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

[Rohit Shinde]

Added the fallthrough comment so that the compiler doesn't emit an error on compiling with the -Wimplicit-fallthrough flag.

Signed off by: Rohit Shinde
---
 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));
 
     case LM_IN_PROGRESS: {
         const QemuOpt *opt;
-- 
2.25.1

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200815130046.5344-1-rohit.shinde12194@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200815130046.5344-1-rohit.shinde12194@gmail.com
Subject: [PATCH] Fixes: Fallthrough warning on line 270 of qemu/qapi/opts-visitor.c

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200815130046.5344-1-rohit.shinde12194@gmail.com -> patchew/20200815130046.5344-1-rohit.shinde12194@gmail.com
Switched to a new branch 'test'
ee13ad0 Fixes: Fallthrough warning on line 270 of qemu/qapi/opts-visitor.c

=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 7 lines checked

Commit ee13ad03a981 (Fixes: Fallthrough warning on line 270 of qemu/qapi/opts-visitor.c) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200815130046.5344-1-rohit.shinde12194@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200815130046.5344-1-rohit.shinde12194@gmail.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qapi/qapi-visit-block.o
  CC      qapi/qapi-visit-char.o
/tmp/qemu-test/src/qapi/opts-visitor.c: In function 'opts_next_list':
/tmp/qemu-test/src/qapi/opts-visitor.c:269:9: error: empty declaration [-Werror]
         __attribute__((fallthrough));
         ^
cc1: all warnings being treated as errors
make: *** [qapi/opts-visitor.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0b358c245bfb4f1ebcc47bd660f243d3', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-hbfp957p/src/docker-src.2020-08-15-11.14.44.15026:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=0b358c245bfb4f1ebcc47bd660f243d3
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-hbfp957p/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m2.249s
user    0m7.870s


The full log is available at
http://patchew.org/logs/20200815130046.5344-1-rohit.shinde12194@gmail.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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;
> 

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

[Rohit Shinde]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

Hey Philippe,

Thanks for the detailed comments! I have a couple of questions.


   1. I'll modify the patch to just include a fallthrough comment instead
   of an attribute. How do I include the v4 version number in the patch? Do I
   erase the last commit on my branch or fork from the master and start the
   work again and label it as v4?
   2. I am trying to find some issues of interest, starting through which I
   can go to bigger contributions. Do you have any suggestions on how I might
   do that? For now, I am trying to tackle the bite sized issues to find my
   way around the code base. I would like to move to substantial contributions.
   3. I have a background in CS theory, but its been 2 years since I
   graduated from my Master's so I am a bit rusty on some stuff. How much CS
   theory (like compilers and OS) do I need to know if I want to contribute?


Thanks,
Rohit.

On Sat, Aug 15, 2020 at 11:24 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> 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;
> >
>
>

[-- Attachment #2: Type: text/html, Size: 4373 bytes --]

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

[Rohit Shinde]

Added the fallthrough comment so that the compiler doesn't emit an error on compiling with the -Wimplicit-fallthrough flag.

Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com>
---
 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));
 
     case LM_IN_PROGRESS: {
         const QemuOpt *opt;
-- 
2.25.1

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

[Philippe Mathieu-Daudé]

When you repost a patch, please add a version. This is patch v2.
Next (once you read my v1 comments) should be v3.

Watch out, v1 correctly Cc'ed the maintainers, this v2 doesn't.

On 8/15/20 5:11 PM, Rohit Shinde wrote:
> Added the fallthrough comment so that the compiler doesn't emit an error on compiling with the -Wimplicit-fallthrough flag.
> 
> Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com>
> ---
>  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));
>  
>      case LM_IN_PROGRESS: {
>          const QemuOpt *opt;
> 

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200815151148.10571-1-rohit.shinde12194@gmail.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qapi/qapi-types-transaction.o
  CC      qapi/qapi-builtin-visit.o
/tmp/qemu-test/src/qapi/opts-visitor.c: In function 'opts_next_list':
/tmp/qemu-test/src/qapi/opts-visitor.c:269:9: error: empty declaration [-Werror]
         __attribute__((fallthrough));
         ^
cc1: all warnings being treated as errors
make: *** [qapi/opts-visitor.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=cbaed9e0baf94ba9adfd067fc89d7df2', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-b0zxj7yg/src/docker-src.2020-08-15-12.10.41.4314:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=cbaed9e0baf94ba9adfd067fc89d7df2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-b0zxj7yg/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    1m57.885s
user    0m6.528s


The full log is available at
http://patchew.org/logs/20200815151148.10571-1-rohit.shinde12194@gmail.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Rohit Shinde]

Added the fallthrough comment so that the compiler doesn't emit an error on compiling with the -Wimplicit-fallthrough flag.

Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com>
---
 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));
 
     case LM_IN_PROGRESS: {
         const QemuOpt *opt;
-- 
2.25.1

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

[Philippe Mathieu-Daudé]

This is v3 with no change w.r.t. v1/v2 (except this time you correctly
Cc'ed the maintainers). Maybe something is wrong in your setup?

On 8/15/20 5:12 PM, Rohit Shinde wrote:
> Added the fallthrough comment so that the compiler doesn't emit an error on compiling with the -Wimplicit-fallthrough flag.
> 
> Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com>
> ---
>  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));
>  
>      case LM_IN_PROGRESS: {
>          const QemuOpt *opt;
> 

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200815151245.10640-1-rohit.shinde12194@gmail.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qapi/qapi-visit-rdma.o
  CC      qapi/qapi-visit-rocker.o
/tmp/qemu-test/src/qapi/opts-visitor.c: In function 'opts_next_list':
/tmp/qemu-test/src/qapi/opts-visitor.c:269:9: error: empty declaration [-Werror]
         __attribute__((fallthrough));
         ^
cc1: all warnings being treated as errors
make: *** [qapi/opts-visitor.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=b5dcb092e02343cf9cd6f3efc4966930', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-vem4s832/src/docker-src.2020-08-15-12.28.53.24667:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=b5dcb092e02343cf9cd6f3efc4966930
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-vem4s832/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    1m44.566s
user    0m9.282s


The full log is available at
http://patchew.org/logs/20200815151245.10640-1-rohit.shinde12194@gmail.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com