| Message ID | 20250323230006.36057-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | system/vl: Tidy up break in QEMU_OPTION_machine case | expand |
On Mon, 24 Mar 2025, Philippe Mathieu-Daudé wrote: > The break in the QEMU_OPTION_machine case is mis-placed. > > Not a big deal, since producing the same outcome, but > suspicious, so put it in the correct place. Why is it misplaced? It's at the end of the block. This swich has other cases that put break outside or inside the block randomly so there's no agreement on that. Changing this one place does not make it consistent. IMO inside the block looks better but I'd align the {} with the case and not indent them but this probably does not worth the effort to clean it. Regards, BALATON Zoltan > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > system/vl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/system/vl.c b/system/vl.c > index ec93988a03a..dbca9ebba4d 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -3409,8 +3409,8 @@ void qemu_init(int argc, char **argv) > machine_help_func(machine_opts_dict); > exit(EXIT_SUCCESS); > } > - break; > } > + break; > case QEMU_OPTION_accel: > accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), > optarg, true); >
On Mon, Mar 24, 2025 at 12:00:06AM +0100, Philippe Mathieu-Daudé wrote: > The break in the QEMU_OPTION_machine case is mis-placed. I think that's largely a bikeshed colouring question. If you look at other places in the outer switch using a block in the case, eg case FOO: { ..... } or case FOO: { ..... } they'll also have 'break' inside the '{}', so either this patch should change all, or change none. > > Not a big deal, since producing the same outcome, but > suspicious, so put it in the correct place. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > system/vl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/system/vl.c b/system/vl.c > index ec93988a03a..dbca9ebba4d 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -3409,8 +3409,8 @@ void qemu_init(int argc, char **argv) > machine_help_func(machine_opts_dict); > exit(EXIT_SUCCESS); > } > - break; > } > + break; > case QEMU_OPTION_accel: > accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), > optarg, true); > -- > 2.47.1 > > With regards, Daniel
On 24/3/25 10:09, Daniel P. Berrangé wrote: > On Mon, Mar 24, 2025 at 12:00:06AM +0100, Philippe Mathieu-Daudé wrote: >> The break in the QEMU_OPTION_machine case is mis-placed. > > I think that's largely a bikeshed colouring question. If you > look at other places in the outer switch using a block in > the case, eg > > case FOO: > { > ..... > } > > or > > case FOO: { > ..... > } > > they'll also have 'break' inside the '{}', so either this patch > should change all, or change none. OK, none then! Thanks :) > >> >> Not a big deal, since producing the same outcome, but >> suspicious, so put it in the correct place. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> system/vl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/system/vl.c b/system/vl.c index ec93988a03a..dbca9ebba4d 100644 --- a/system/vl.c +++ b/system/vl.c @@ -3409,8 +3409,8 @@ void qemu_init(int argc, char **argv) machine_help_func(machine_opts_dict); exit(EXIT_SUCCESS); } - break; } + break; case QEMU_OPTION_accel: accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), optarg, true);
The break in the QEMU_OPTION_machine case is mis-placed. Not a big deal, since producing the same outcome, but suspicious, so put it in the correct place. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- system/vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)