| Message ID | 20200815130046.5344-1-rohit.shinde12194@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes: Fallthrough warning on line 270 of qemu/qapi/opts-visitor.c | expand |
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
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
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; >
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; > > > >
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;