Re: [Qemu-devel] [PATCH 2/7] iohandlers: Introduce a new API

[Avi Kivity <avi@redhat.com>]

On 02/22/2011 05:00 PM, Anthony Liguori wrote:
>>
>> Drop the opaque, instead put the IOHandler in there (or maybe the 
>> CharDev?) and use container_of().
>
>
> You know, I'm not there that this is automatically better.

Why not?  One less unsafe cast.

>
>>> +
>>> +int assign_iohandler(int fd, IOAllHandler *handler, unsigned int mask,
>>> +                     void *opaque);
>>> +int remove_iohandler(int fd);
>>> +int update_fd_mask(int fd, unsigned int mask);
>>> +int get_fd_mask(int fd, unsigned int *mask);
>>> +
>>
>> iohandler_init(IOHandler *ioh, int fd, IOEventHandler *handler, 
>> unsigned mask)
>> iohandler_del(IOHandler *ioh)
>> iohandler_set_event_mask(IOHandler *ioh, unsigned mask)
>> iohandler_event_mask(IOHandler *ioh)
>>
>> No opaques, use an object as a key so you don't have to search for it.
>
> I don't think there should be a setter/getter for the event mask.  
> Just delete the handler and re-add it.
>

It's more epoll() compatible to update it.  I'm fine with reregistering 
though.

> Why is there an IOHandler and an IOEventHandler argument for init?

IOHandler is the object, IOEventHandler is the function call.

Basically this is a 1:1 translation of

class IOHandler {
public:
     IOHandler(int fd, unsigned mask);
     ~IOHandler();
     virtual void io_event_handler() = 0;
     unsigned event_mask();
     void set_event_mask(unsigned mask);
}

class MyClient : private IOHandler {
     virtual void io_event_handler();
}

where the compiler automatically generates the container_of() to get to 
the actual MyClient pointer.