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

[Avi Kivity]

On 02/22/2011 08:21 PM, Anthony Liguori wrote:
> On 02/22/2011 10:21 AM, Avi Kivity wrote:
> > 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.
>
> It turns out there are a lot of cases in QEMU where there isn't an 
> obvious data structure to piggy back on.

Examples?

In that case you can always make a structure with a void * and a 
IOHandler, and you get your opaque.  But I don't really see how it can 
happen, the void * always points to a structure (or is ignored if 
there's a static instance), and you can add that IOHandler to the structure.

> > > Why is there an IOHandler and an IOEventHandler argument for init?
>
> So wouldn't you set the IOEventHandler directly in the IOHandler as 
> part of initialization?  IOW:
>
> struct MyDevice {
>     IOHandler iohandler;
> };
>
> iohandler.func = my_io_function;
> io_handler_init(&iohandler, fd, event_mask);
>
> That's the paradigm I'm used to at least.

You mean instead of passing .func to io_handler_init()?  I slightly 
prefer my version (less chance to forget the assignment) but I'm okay 
with either.

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