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<amit.s...@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.
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