From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58062) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPCZE-0005b2-Tm for qemu-devel@nongnu.org; Mon, 08 Apr 2013 09:57:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPCZ8-0006pl-Kg for qemu-devel@nongnu.org; Mon, 08 Apr 2013 09:57:04 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:51606) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPCZ8-0006UD-3S for qemu-devel@nongnu.org; Mon, 08 Apr 2013 09:56:58 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 8 Apr 2013 23:50:39 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id DBFE02BB0023 for ; Mon, 8 Apr 2013 23:56:11 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r38DgkJX459142 for ; Mon, 8 Apr 2013 23:42:47 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r38DuBRV014209 for ; Mon, 8 Apr 2013 23:56:11 +1000 From: Anthony Liguori In-Reply-To: <1365426195-12596-1-git-send-email-pbonzini@redhat.com> References: <1365426195-12596-1-git-send-email-pbonzini@redhat.com> Date: Mon, 08 Apr 2013 08:56:01 -0500 Message-ID: <87a9p9gm8e.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH] qemu-char: really fix behavior on can_read = 0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: amit.shah@redhat.com, peter.crosthwaite@xilinx.com Paolo Bonzini writes: > I misread the glib manual, g_source_remove does not let you re-attach > the source later. This behavior (called "blocking" the source in glib) > is present in glib's source code, but private and not available outside > glib; hence, we have to resort to re-creating the source every time. > > In fact, g_source_remove and g_source_destroy are the same thing, > except g_source_destroy is O(1) while g_source_remove scans a potentially > very long list of GSources in the current main loop. Ugh. Better > use g_source_destroy explicitly, and leave "tags" to those dummies who > cannot track their pointers' lifetimes. > > Signed-off-by: Paolo Bonzini Tested-by: Anthony Liguori Regards, Anthony Liguori > --- > qemu-char.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index dd410ce..eae17fc 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -596,9 +596,11 @@ typedef struct IOWatchPoll > { > GSource parent; > > + GIOChannel *channel; > GSource *src; > > IOCanReadHandler *fd_can_read; > + GSourceFunc fd_read; > void *opaque; > } IOWatchPoll; > > @@ -611,15 +613,19 @@ static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_) > { > IOWatchPoll *iwp = io_watch_poll_from_source(source); > bool now_active = iwp->fd_can_read(iwp->opaque) > 0; > - bool was_active = g_source_get_context(iwp->src) != NULL; > + bool was_active = iwp->src != NULL; > if (was_active == now_active) { > return FALSE; > } > > if (now_active) { > + iwp->src = g_io_create_watch(iwp->channel, G_IO_IN | G_IO_ERR | G_IO_HUP); > + g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL); > g_source_attach(iwp->src, NULL); > } else { > - g_source_remove(g_source_get_id(iwp->src)); > + g_source_destroy(iwp->src); > + g_source_unref(iwp->src); > + iwp->src = NULL; > } > return FALSE; > } > @@ -638,7 +644,9 @@ static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback, > static void io_watch_poll_finalize(GSource *source) > { > IOWatchPoll *iwp = io_watch_poll_from_source(source); > + g_source_destroy(iwp->src); > g_source_unref(iwp->src); > + iwp->src = NULL; > } > > static GSourceFuncs io_watch_poll_funcs = { > @@ -659,8 +667,9 @@ static guint io_add_watch_poll(GIOChannel *channel, > iwp = (IOWatchPoll *) g_source_new(&io_watch_poll_funcs, sizeof(IOWatchPoll)); > iwp->fd_can_read = fd_can_read; > iwp->opaque = user_data; > - iwp->src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP); > - g_source_set_callback(iwp->src, (GSourceFunc)fd_read, user_data, NULL); > + iwp->channel = channel; > + iwp->fd_read = (GSourceFunc) fd_read; > + iwp->src = NULL; > > return g_source_attach(&iwp->parent, NULL); > } > -- > 1.8.1.4