[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initi
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers |
Date: |
Fri, 10 Feb 2012 19:08:36 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 |
Am 10.02.2012 18:22, schrieb Anthony Liguori:
> 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<address@hidden>
>>>
>>> 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.
The obvious next step is to replace
CharDriverState.chr_can_read/read/event with a CharDriverState.ops that
refers to a CharDriverHandler struct, and suddenly you have a pattern
that is used a lot in QEMU (and that indeed increases readability in my
opinion).
Kevin