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

[Chen Gang]

On 04/01/2014 04:13 PM, Markus Armbruster wrote:
> 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 :)
> 

Yeah, more discussions, and more thinking of, before implementations.

>> [...]
>>>> @@ -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.
> 

I guess, the root cause is the main() is too long, and need be split
into several small functions, then use 'return' instead of exit(), and
then remove "'break' after exit()".

And after split into small functions, we also can by pass the several
"{...}" styles within switch() in main() -- let the related code block
be in individual functions.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed