| Message ID | 20200816023127.22268-1-rohit.shinde12194@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] qapi/opts-visitor: Fixed fallthrough compiler warning | expand |
On 8/15/20 7:31 PM, Rohit Shinde wrote: > /* range has been completed, fall through in order to pop option */ > - __attribute__((fallthrough)); > + /* fallthrough */ (1) Any patch should not be relative to your own v3. (2) The previous line already contains the words "fall through", so what is it that you are trying to fix? r~
Hey Richard, 1. So I should fork off the master again? I am a bit unclear on the workflow, since this is my first doing patches via format-patch and send-email so I am making mistakes. 2. I just checked and my version of the code doesn't contain that line, so I am unsure on how that line got there. I was trying to fix the compiler warnings. Could you please guide me on how I create the next version of a patch? Thanks, Rohit. On Sun, Aug 16, 2020 at 12:03 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/15/20 7:31 PM, Rohit Shinde wrote: > > /* range has been completed, fall through in order to pop > option */ > > - __attribute__((fallthrough)); > > + /* fallthrough */ > > (1) Any patch should not be relative to your own v3. > (2) The previous line already contains the words "fall through", > so what is it that you are trying to fix? > > > r~ >
On 8/16/20 9:55 AM, Rohit Shinde wrote: > Hey Richard, > > 1. So I should fork off the master again? Yes. There need to be special circumstances for not posting a patch set relative to master, and even then your cover letter would need to detail against what base the patch set applies. > 2. I just checked and my version of the code doesn't contain that line, so I > am unsure on how that line got there. I was trying to fix the compiler > warnings. Could you please guide me on how I create the next version of a > patch? That makes no sense at all. The line was added at the same time as the code above it, in d8754f40acb. There should be nothing to fix. r~
On Sun, Aug 16, 2020 at 6:52 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/16/20 9:55 AM, Rohit Shinde wrote: > > Hey Richard, > > > > 1. So I should fork off the master again? > > Yes. > > There need to be special circumstances for not posting a patch set > relative to > master, and even then your cover letter would need to detail against what > base > the patch set applies. > > > 2. I just checked and my version of the code doesn't contain that line, > so I > > am unsure on how that line got there. I was trying to fix the > compiler > > warnings. Could you please guide me on how I create the next version > of a > > patch? > > That makes no sense at all. The line was added at the same time as the > code > above it, in d8754f40acb. > I misread the comment. The comment /* fallthrough */ was meant to stop the compiler warning from occurring. I am trying to complete the bite sized task mentioned here <https://wiki.qemu.org/Contribute/BiteSizedTasks> under "Compiler driven cleanups". I wanted to take that up to get more familiar with the codebase. > > There should be nothing to fix. > > > r~ >
On 8/16/20 5:57 PM, Rohit Shinde wrote: > I misread the comment. The comment /* fallthrough */ was meant to stop the > compiler warning from occurring. I am trying to complete the bite sized task > mentioned here <https://wiki.qemu.org/Contribute/BiteSizedTasks> under > "Compiler driven cleanups". I wanted to take that up to get more familiar with > the codebase. Fine, but the current comment matches the compiler regexp, so this instance isn't part of that cleanup. r~
On Sun, Aug 16, 2020 at 11:26 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/16/20 5:57 PM, Rohit Shinde wrote: > > I misread the comment. The comment /* fallthrough */ was meant to stop > the > > compiler warning from occurring. I am trying to complete the bite sized > task > > mentioned here <https://wiki.qemu.org/Contribute/BiteSizedTasks> under > > "Compiler driven cleanups". I wanted to take that up to get more > familiar with > > the codebase. > > Fine, but the current comment matches the compiler regexp, so this instance > isn't part of that cleanup. > But when I was compiling with the -Wimplicit-fallthrough option, the compiler gave an error at that very line. That's why I came up on that specific line in that file first. I am pasting the error that I got below: home/rohit/Desktop/open-source/qemu/qapi/opts-visitor.c: In function ‘opts_next_list’: /home/rohit/Desktop/open-source/qemu/qapi/opts-visitor.c:267:23: error: this statement may fall through [-Werror=implicit-fallthrough=] 267 | ov->list_mode = LM_IN_PROGRESS; | ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/rohit/Desktop/open-source/qemu/qapi/opts-visitor.c:270:5: note: here 270 | case LM_IN_PROGRESS: { | ^~~~ Sending it again, because I forgot to reply all. Thanks, Rohit. > > > r~ > >
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 43cf60d3a0..3422ff265e 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -266,7 +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)); + /* fallthrough */ case LM_IN_PROGRESS: { const QemuOpt *opt;
Added fallthrough comment on line 270 to fix compiler warning Signed-off-by: Rohit Shinde <rohit.shinde12194@gmail.com> --- qapi/opts-visitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)