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

[Avi Kivity]

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.