Re: [Qemu-devel] [PATCH trival] vl.c: clean up code

["Alex Bennée" <alex.bennee@linaro.org>]

Chen Gang <gang.chen.5i5j@gmail.com> writes:

> Hello Maintainers:
>
> In main switch of main(), it contents several styles for "{...}" code block.
>
> If it is necessary to use unique style within a function, please let me
> know, I will/should clean up it. And also better to tell me which style
> we need choose -- for me, I don't know which style is the best to
> Qemu.

The correct block style is described in CODING_STYLE Section 4. However
chunks of the code base violate the style guidelines and should be
cleaned up as other fixes are made.

>
>
> Thanks.
>
> On 03/30/2014 10:34 PM, Chen Gang wrote:
>> in get_boot_device()
>> 
>>  - remove 'res' to simplify code
>> 
>> in main():
>> 
>>  - remove useless 'continue'.
>> 
>>  - in main switch():
>> 
>>    - remove or adjust all useless 'break'.
>> 
>>    - remove useless '{' and '}'.
>> 
>>  - use assignment directly to replace useless 'args'
>>    (which is defined in the middle of code block).
>> 
>> 
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  vl.c | 36 +++++++++++++-----------------------
>>  1 file changed, 13 insertions(+), 23 deletions(-)
>> 
>> diff --git a/vl.c b/vl.c
>> index 9975e5a..9c733cb 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1188,18 +1188,16 @@ DeviceState *get_boot_device(uint32_t position)
>>  {
>>      uint32_t counter = 0;
>>      FWBootEntry *i = NULL;
>> -    DeviceState *res = NULL;
>>  
>>      if (!QTAILQ_EMPTY(&fw_boot_order)) {
>>          QTAILQ_FOREACH(i, &fw_boot_order, link) {
>>              if (counter == position) {
>> -                res = i->dev;
>> -                break;
>> +                return i->dev;
>>              }
>>              counter++;
>>          }
>>      }
>> -    return res;
>> +    return NULL;
>>  }
>>  
>>  /*
>> @@ -3034,7 +3032,6 @@ int main(int argc, char **argv, char **envp)
>>          if (argv[optind][0] != '-') {
>>              /* disk image */
>>              optind++;
>> -            continue;
>>          } else {
>>              const QEMUOption *popt;
>>  
>> @@ -3204,11 +3201,11 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_curses:
>>  #ifdef CONFIG_CURSES
>>                  display_type = DT_CURSES;
>> +                break;
>>  #else
>>                  fprintf(stderr, "Curses support is disabled\n");
>>                  exit(1);
>>  #endif
>> -                break;
>>              case QEMU_OPTION_portrait:
>>                  graphic_rotate = 90;
>>                  break;
>> @@ -3286,7 +3283,6 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_audio_help:
>>                  AUD_help ();
>>                  exit (0);
>> -                break;
>>              case QEMU_OPTION_soundhw:
>>                  select_soundhw (optarg);
>>                  break;
>> @@ -3296,7 +3292,6 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_version:
>>                  version();
>>                  exit(0);
>> -                break;
>>              case QEMU_OPTION_m: {
>>                  int64_t value;
>>                  uint64_t sz;
>> @@ -3638,11 +3633,10 @@ int main(int argc, char **argv, char **envp)
>>                  olist = qemu_find_opts("machine");
>>                  qemu_opts_parse(olist, "accel=tcg", 0);
>>                  break;
>> -            case QEMU_OPTION_no_kvm_pit: {
>> +            case QEMU_OPTION_no_kvm_pit:
>>                  fprintf(stderr, "Warning: KVM PIT can no longer be disabled "
>>                                  "separately.\n");
>>                  break;
>> -            }
>>              case QEMU_OPTION_no_kvm_pit_reinjection: {
>>                  static GlobalProperty kvm_pit_lost_tick_policy[] = {
>>                      {
>> @@ -3681,11 +3675,11 @@ int main(int argc, char **argv, char **envp)
>>  #ifdef CONFIG_VNC
>>                  display_remote++;
>>                  vnc_display = optarg;
>> +                break;
>>  #else
>>                  fprintf(stderr, "VNC support is disabled\n");
>>                  exit(1);
>>  #endif
>> -                break;
>>              case QEMU_OPTION_no_acpi:
>>                  acpi_enabled = 0;
>>                  break;
>> @@ -3811,7 +3805,6 @@ int main(int argc, char **argv, char **envp)
>>                  xen_mode = XEN_ATTACH;
>>                  break;
>>              case QEMU_OPTION_trace:
>> -            {
>>                  opts = qemu_opts_parse(qemu_find_opts("trace"), optarg, 0);
>>                  if (!opts) {
>>                      exit(1);
>> @@ -3819,7 +3812,6 @@ int main(int argc, char **argv, char **envp)
>>                  trace_events = qemu_opt_get(opts, "events");
>>                  trace_file = qemu_opt_get(opts, "file");
>>                  break;
>> -            }
>>              case QEMU_OPTION_readconfig:
>>                  {
>>                      int ret = qemu_read_config_file(optarg);
>> @@ -3876,12 +3868,12 @@ int main(int argc, char **argv, char **envp)
>>                  if (!opts) {
>>                      exit(1);
>>                  }
>> +                break;
>>  #else
>>                  error_report("File descriptor passing is disabled on this "
>>                               "platform");
>>                  exit(1);
>>  #endif
>> -                break;
>>              case QEMU_OPTION_object:
>>                  opts = qemu_opts_parse(qemu_find_opts("object"), optarg, 1);
>>                  if (!opts) {
>> @@ -4371,15 +4363,13 @@ 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.machine = machine;
>> +    current_machine->init_args.ram_size = ram_size;
>> +    current_machine->init_args.boot_order = boot_order;
>> +    current_machine->init_args.kernel_filename = kernel_filename;
>> +    current_machine->init_args.kernel_cmdline = kernel_cmdline;
>> +    current_machine->init_args.initrd_filename = initrd_filename;
>> +    current_machine->init_args.cpu_model = cpu_model;
>>      machine->init(&current_machine->init_args);
>>  
>>      audio_init();
>> 

-- 
Alex Bennée