From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=51854 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PctsH-0008BS-Nq
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 01:08:02 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1PctsG-0007rD-7i
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 01:08:01 -0500
Received: from mx1.redhat.com ([209.132.183.28]:51218)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1PctsF-0007r4-TP
	for qemu-devel@nongnu.org; Wed, 12 Jan 2011 01:08:00 -0500
Date: Wed, 12 Jan 2011 11:37:45 +0530
From: Amit Shah <amit.shah@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to
	initialise chardev handlers
Message-ID: <20110112060743.GB12381@amit-x200.redhat.com>
References: <cover.1294743490.git.amit.shah@redhat.com>
	<edcae4488c7a4e27a9065ebe01044f215974b5ce.1294743490.git.amit.shah@redhat.com>
	<AANLkTikxET=y=KG422qFpxKViN3REjwQnj7O++UM_Vna@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <AANLkTikxET=y=KG422qFpxKViN3REjwQnj7O++UM_Vna@mail.gmail.com>
Content-Transfer-Encoding: quoted-printable
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: Blue Swirl <blauwirbel@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>, qemu list <qemu-devel@nongnu.org>, Paul Brook <paul@codesourcery.com>

On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote:
> > +static QemuChrHandlers gdb_handlers =3D {
> > + =A0 =A0.fd_can_read =3D gdb_chr_can_receive,
> > + =A0 =A0.fd_read =3D gdb_chr_receive,
> > + =A0 =A0.fd_event =3D gdb_chr_event,
> > +};
>=20
> 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, in=
t event)
> > =A0 =A0 =A0 =A0 s->chr_send_event(s, event);
> > =A0}
> >
> > +static QemuChrHandlers null_handlers =3D {
> > + =A0 =A0/* All handlers are initialised to NULL */
> > +};
> > +
> > =A0void qemu_chr_add_handlers(CharDriverState *s,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 IOCanReadHandle=
r *fd_can_read,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 IOReadHandler *=
fd_read,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 IOEventHandler =
*fd_event,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 void *opaque)
> > -{
> > - =A0 =A0s->chr_can_read =3D fd_can_read;
> > - =A0 =A0s->chr_read =3D fd_read;
> > - =A0 =A0s->chr_event =3D fd_event;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 QemuChrHandlers=
 *handlers, void *opaque)
> > +{
>=20
> Here we could also check if (!s) and return if so. This would simplify
> the callers a bit.

Simplified in what way?

		Amit