From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=50595 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Pd510-00058U-Fu
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 13:01:47 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mdroth@linux.vnet.ibm.com>) id 1Pd50x-00080z-Sn
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 13:01:44 -0500
Received: from e4.ny.us.ibm.com ([32.97.182.144]:41373)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mdroth@linux.vnet.ibm.com>) id 1Pd50x-00080Q-QT
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 13:01:43 -0500
Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56])
	by e4.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p0CHeJxn002060
	for <qemu-devel@nongnu.org>; Wed, 12 Jan 2011 12:43:47 -0500
Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147])
	by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 8F4FA7281B6
	for <qemu-devel@nongnu.org>; Wed, 12 Jan 2011 13:01:10 -0500 (EST)
Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216])
	by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id
	p0CI19f7700542
	for <qemu-devel@nongnu.org>; Wed, 12 Jan 2011 13:01:09 -0500
Received: from d01av02.pok.ibm.com (loopback [127.0.0.1])
	by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id
	p0CI19iP029518
	for <qemu-devel@nongnu.org>; Wed, 12 Jan 2011 16:01:09 -0200
Message-ID: <4D2DEC5F.6050708@linux.vnet.ibm.com>
Date: Wed, 12 Jan 2011 12:01:03 -0600
From: Michael Roth <mdroth@linux.vnet.ibm.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to
	initialise chardev handlers
References: <cover.1294743490.git.amit.shah@redhat.com>
	<edcae4488c7a4e27a9065ebe01044f215974b5ce.1294743490.git.amit.shah@redhat.com>	<AANLkTikxET=y=KG422qFpxKViN3REjwQnj7O++UM_Vna@mail.gmail.com>
	<20110112060743.GB12381@amit-x200.redhat.com>
In-Reply-To: <20110112060743.GB12381@amit-x200.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: Amit Shah <amit.shah@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>, Paul Brook <paul@codesourcery.com>, Gerd Hoffmann <kraxel@redhat.com>, qemu list <qemu-devel@nongnu.org>

On 01/12/2011 12:07 AM, Amit Shah wrote:
> On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote:
>>> +static QemuChrHandlers gdb_handlers = {
>>> +    .fd_can_read = gdb_chr_can_receive,
>>> +    .fd_read = gdb_chr_receive,
>>> +    .fd_event = gdb_chr_event,
>>> +};
>>
>> These structures should be const.
>
> Hm, I had that but looks like it got lost in some rebase.  Added again.
>
>>> @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int event)
>>>          s->chr_send_event(s, event);
>>>   }
>>>
>>> +static QemuChrHandlers null_handlers = {
>>> +    /* All handlers are initialised to NULL */
>>> +};
>>> +
>>>   void qemu_chr_add_handlers(CharDriverState *s,
>>> -                           IOCanReadHandler *fd_can_read,
>>> -                           IOReadHandler *fd_read,
>>> -                           IOEventHandler *fd_event,
>>> -                           void *opaque)
>>> -{
>>> -    s->chr_can_read = fd_can_read;
>>> -    s->chr_read = fd_read;
>>> -    s->chr_event = fd_event;
>>> +                           QemuChrHandlers *handlers, void *opaque)
>>> +{
>>
>> Here we could also check if (!s) and return if so. This would simplify
>> the callers a bit.
>
> Simplified in what way?

I assume for reducing the need to have to check s->chr != NULL everytime 
beforehand. It's safer and would save a lot on repetitive code as well.

>
> 		Amit
>