From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=55008 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PrwrT-0002sR-Kn
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 13:21:24 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1PrwrS-0006ku-ER
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 13:21:23 -0500
Received: from mail-vw0-f45.google.com ([209.85.212.45]:58219)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1PrwrS-0006kp-Al
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 13:21:22 -0500
Received: by vws19 with SMTP id 19so3050139vws.4
	for <qemu-devel@nongnu.org>; Tue, 22 Feb 2011 10:21:22 -0800 (PST)
Message-ID: <4D63FEA4.1000002@codemonkey.ws>
Date: Tue, 22 Feb 2011 12:21:24 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/7] iohandlers: Introduce a new API
References: <cover.1298369272.git.amit.shah@redhat.com>	<3d41a2ca603b8132e2496976e89fca09fc246683.1298369272.git.amit.shah@redhat.com>	<4D63B9E2.5060201@redhat.com>
	<4D63CF82.8060006@codemonkey.ws> <4D63E299.8070808@redhat.com>
In-Reply-To: <4D63E299.8070808@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Avi Kivity <avi@redhat.com>
Cc: Amit Shah <amit.shah@redhat.com>, qemu list <qemu-devel@nongnu.org>, Gerd Hoffmann <kraxel@redhat.com>

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.

>> 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.

Regards,

Anthony Liguori

> 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.
>