Re: [PATCH 2/5] KVM: Add irq ack notifier list

[Avi Kivity <avi@qumranet.com>]

Amit Shah wrote:
>>> 'opaque' fields can be later made to point to other structures
>>> without changing the structure itself. This is an advantage. Will
>>> the kvm_pic struct be needed to change in the future? Very
>>> unlikely. So we can rename it to struct kvm *, however, that gives
>>> us no real benefit as against opaque (just readability).
>>>       
>> Yes, the readability...
>>
>> I think people would be very curious about why
>>
>> void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi)
>>
>> got a irq_request_opaque as a parameter. It's more like a hack, which
>> is not my meaning...
>>
>> Anyway, it's trivial one and just a coding style. :)
>>     
>
> However, this idiom is used in quite a few places in the kernel already so it 
> shouldn't come as a big surprise to someone reading the code.
>   

Void pointers are useful where there are multiple clients for a service 
which needs a callback.  When there is just one client (and not because 
no one has written another, but because logically there is only one 
client), specifying it explicity is better.

An alternative to void pointers (which I prefer) is to embed the 
structure that holds the callback within the structure that needs to be 
passed to the callback, and use container_of().  This saves the space to 
store the void pointer, and is also cleaner, IMO.

-- 
error compiling committee.c: too many arguments to function