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

[Markus Armbruster]

Chen Gang <address@hidden> writes:

> On 03/31/2014 11:49 PM, Markus Armbruster wrote:
>> Chen Gang <address@hidden> writes:
>> 
>>> 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).
>> 
>> Suggest to have one patch per item in this list.
>> 
>
> OK, thanks. I will/should send again in this week (within 2014-04-06).

No rush :)

> [...]
>>> @@ -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;
>> 
>> I'm not sure eliding the break after exit is worthwhile.
>> 
>> In theory, it could cause false positives with tools smart enough to
>> diagnose fall through without comment, but too stupid to see that exit()
>> never returns.  No idea whether such tools exist.
>> 
>> The missing break might give human readers slight pause, until they
>> recognize exit.  Especially with such ifdeffery.
>>
>
> That sounds reasonable to me.
>
> "removing 'break' after exit()" will not be good for C code readers:
> exit() is not like 'return' which can get enough focus by C code readers
> (especially in 'vim' or other latest editor).
>
> How about to let 'return' instead of exit() in main(), and also remove
> useless 'break'? Welcome any additional suggestions, discussions or
> completions, thanks.

When main() is short, return instead of exit() is just fine with me.
But when it's as long as this one, I prefer exit() to make the process
termination locally obvious.

I think the code is okay as it is.

[...]