| Message ID | 5343E5ED.2040906@gmail.com |
|---|---|
| State | New |
| Headers | show |
Chen Gang <gang.chen.5i5j@gmail.com> writes: > For C code, it does not recommend to define a local variable in the > middle of code block without "{...}". The original author may want to > zero members not mentioned in structure assignment. > > So recommend to use structure initializing block "{...}" for structure > assignment in the middle of code block. > > And at present, we can assume that all related gcc versions will be > latest enough to notice about this grammar. Commit message is a bit confusing :) I'd write something like this: vl: Eliminate a superfluous local variable CODING_STYLE frowns upon mixing declarations and statements. main() has such a declaration. Clean up by eliminating the variable. > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > vl.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/vl.c b/vl.c > index 377f962..d381443 100644 > --- a/vl.c > +++ b/vl.c > @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) > > qdev_machine_init(); > > - QEMUMachineInitArgs args = { .machine = machine, > - .ram_size = ram_size, > - .boot_order = boot_order, > - .kernel_filename = kernel_filename, > - .kernel_cmdline = kernel_cmdline, > - .initrd_filename = initrd_filename, > - .cpu_model = cpu_model }; > - > - current_machine->init_args = args; > + current_machine->init_args = (QEMUMachineInitArgs) { > + .machine = machine, > + .ram_size = ram_size, > + .boot_order = boot_order, > + .kernel_filename = kernel_filename, > + .kernel_cmdline = kernel_cmdline, > + .initrd_filename = initrd_filename, > + .cpu_model = cpu_model }; > + > machine->init(¤t_machine->init_args); > > audio_init(); Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 04/15/2014 04:56 PM, Markus Armbruster wrote: > Chen Gang <gang.chen.5i5j@gmail.com> writes: > >> For C code, it does not recommend to define a local variable in the >> middle of code block without "{...}". The original author may want to >> zero members not mentioned in structure assignment. >> >> So recommend to use structure initializing block "{...}" for structure >> assignment in the middle of code block. >> >> And at present, we can assume that all related gcc versions will be >> latest enough to notice about this grammar. > > Commit message is a bit confusing :) I'd write something like this: > > vl: Eliminate a superfluous local variable > > CODING_STYLE frowns upon mixing declarations and statements. main() > has such a declaration. Clean up by eliminating the variable. > That sounds fine to me. And excuse me, my English is not quite well (so may write confusing and redundancy comments) Thanks. >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> >> --- >> vl.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 377f962..d381443 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) >> >> qdev_machine_init(); >> >> - QEMUMachineInitArgs args = { .machine = machine, >> - .ram_size = ram_size, >> - .boot_order = boot_order, >> - .kernel_filename = kernel_filename, >> - .kernel_cmdline = kernel_cmdline, >> - .initrd_filename = initrd_filename, >> - .cpu_model = cpu_model }; >> - >> - current_machine->init_args = args; >> + current_machine->init_args = (QEMUMachineInitArgs) { >> + .machine = machine, >> + .ram_size = ram_size, >> + .boot_order = boot_order, >> + .kernel_filename = kernel_filename, >> + .kernel_cmdline = kernel_cmdline, >> + .initrd_filename = initrd_filename, >> + .cpu_model = cpu_model }; >> + >> machine->init(¤t_machine->init_args); >> >> audio_init(); > > Reviewed-by: Markus Armbruster <armbru@redhat.com> >
On Tue, Apr 8, 2014 at 10:05 PM, Chen Gang <gang.chen.5i5j@gmail.com> wrote: > For C code, it does not recommend to define a local variable in the > middle of code block without "{...}". The original author may want to > zero members not mentioned in structure assignment. > > So recommend to use structure initializing block "{...}" for structure > assignment in the middle of code block. > > And at present, we can assume that all related gcc versions will be > latest enough to notice about this grammar. > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> With Markus' suggest commit message fixup: Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > vl.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/vl.c b/vl.c > index 377f962..d381443 100644 > --- a/vl.c > +++ b/vl.c > @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) > > qdev_machine_init(); > > - QEMUMachineInitArgs args = { .machine = machine, > - .ram_size = ram_size, > - .boot_order = boot_order, > - .kernel_filename = kernel_filename, > - .kernel_cmdline = kernel_cmdline, > - .initrd_filename = initrd_filename, > - .cpu_model = cpu_model }; > - > - current_machine->init_args = args; > + current_machine->init_args = (QEMUMachineInitArgs) { > + .machine = machine, > + .ram_size = ram_size, > + .boot_order = boot_order, > + .kernel_filename = kernel_filename, > + .kernel_cmdline = kernel_cmdline, > + .initrd_filename = initrd_filename, > + .cpu_model = cpu_model }; > + > machine->init(¤t_machine->init_args); > > audio_init(); > -- > 1.7.9.5 >
On 04/15/2014 08:29 PM, Peter Crosthwaite wrote: > On Tue, Apr 8, 2014 at 10:05 PM, Chen Gang <gang.chen.5i5j@gmail.com> wrote: >> For C code, it does not recommend to define a local variable in the >> middle of code block without "{...}". The original author may want to >> zero members not mentioned in structure assignment. >> >> So recommend to use structure initializing block "{...}" for structure >> assignment in the middle of code block. >> >> And at present, we can assume that all related gcc versions will be >> latest enough to notice about this grammar. >> >> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > > With Markus' suggest commit message fixup: > > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > OK, thanks. I will use the new commit message in the patch v2. >> --- >> vl.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 377f962..d381443 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) >> >> qdev_machine_init(); >> >> - QEMUMachineInitArgs args = { .machine = machine, >> - .ram_size = ram_size, >> - .boot_order = boot_order, >> - .kernel_filename = kernel_filename, >> - .kernel_cmdline = kernel_cmdline, >> - .initrd_filename = initrd_filename, >> - .cpu_model = cpu_model }; >> - >> - current_machine->init_args = args; >> + current_machine->init_args = (QEMUMachineInitArgs) { >> + .machine = machine, >> + .ram_size = ram_size, >> + .boot_order = boot_order, >> + .kernel_filename = kernel_filename, >> + .kernel_cmdline = kernel_cmdline, >> + .initrd_filename = initrd_filename, >> + .cpu_model = cpu_model }; >> + >> machine->init(¤t_machine->init_args); >> >> audio_init(); >> -- >> 1.7.9.5 >>
diff --git a/vl.c b/vl.c index 377f962..d381443 100644 --- a/vl.c +++ b/vl.c @@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp) qdev_machine_init(); - QEMUMachineInitArgs args = { .machine = machine, - .ram_size = ram_size, - .boot_order = boot_order, - .kernel_filename = kernel_filename, - .kernel_cmdline = kernel_cmdline, - .initrd_filename = initrd_filename, - .cpu_model = cpu_model }; - - current_machine->init_args = args; + current_machine->init_args = (QEMUMachineInitArgs) { + .machine = machine, + .ram_size = ram_size, + .boot_order = boot_order, + .kernel_filename = kernel_filename, + .kernel_cmdline = kernel_cmdline, + .initrd_filename = initrd_filename, + .cpu_model = cpu_model }; + machine->init(¤t_machine->init_args); audio_init();
For C code, it does not recommend to define a local variable in the middle of code block without "{...}". The original author may want to zero members not mentioned in structure assignment. So recommend to use structure initializing block "{...}" for structure assignment in the middle of code block. And at present, we can assume that all related gcc versions will be latest enough to notice about this grammar. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- vl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)