Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/10/2012 11:09 AM, Amit Shah wrote:
> On (Fri) 10 Feb 2012 [07:28:19], Anthony Liguori wrote:
>> On 02/10/2012 07:19 AM, Amit Shah wrote:
>>> Instead of passing each handler in the qemu_add_handlers() function,
>>> create a struct of handlers that can be passed to the function instead.
>>>
>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>
>> Why?
>
> It's a good cleanup.
>
>> It's not a win in terms of code size.  If you plan on introducing
>> additional handlers, perhaps you should include this in that series
>> where it's more appropriately justified.
>>
>> As a change on it's own, it doesn't seem to make a lot of sense.
>
> Makes the code much easier to look at.  Can't really compare on code
> size, since there's zero change in the resulting binary, but the code
> just becomes more readable and manageable.

It's not more readable IMHO.  You've taken function call arguments from the 
place they naturally belong (in the function call) and placed them somewhere else.

More importantly, this isn't a pattern we use in QEMU anywhere.

Regards,

Anthony Liguori

>
> 		Amit