Chen Gang <gang.chen.5...@gmail.com> writes:
> On 03/31/2014 11:49 PM, Markus Armbruster wrote:
>> Chen Gang <gang.chen.5...@gmail.com> 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.
[...]
