From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUyHB-0000vs-HC for qemu-devel@nongnu.org; Wed, 03 Aug 2016 11:40:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUyH5-0003V2-Fr for qemu-devel@nongnu.org; Wed, 03 Aug 2016 11:40:08 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:42035) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUyH5-0003Uf-6k for qemu-devel@nongnu.org; Wed, 03 Aug 2016 11:40:03 -0400 Date: Wed, 3 Aug 2016 11:40:01 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <1524798213.862554.1470238801920.JavaMail.zimbra@redhat.com> In-Reply-To: References: <20160803145541.15355-1-marcandre.lureau@redhat.com> <20160803145541.15355-20-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.7 v3 19/36] char: free the tcp connection data when closing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: marcandre lureau , qemu-devel@nongnu.org, eblake@redhat.com Hi ----- Original Message ----- > > > On 03/08/2016 16:55, marcandre.lureau@redhat.com wrote: > > @@ -2851,11 +2851,6 @@ static void tcp_chr_disconnect(CharDriverState *chr) > > return; > > } > > > > - s->connected = 0; > > - if (s->listen_ioc) { > > - s->listen_tag = qio_channel_add_watch( > > - QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, > > NULL); > > - } > > tcp_set_msgfds(chr, NULL, 0); > > remove_fd_in_watch(chr); > > object_unref(OBJECT(s->sioc)); > > @@ -2863,6 +2858,24 @@ static void tcp_chr_disconnect(CharDriverState *chr) > > object_unref(OBJECT(s->ioc)); > > s->ioc = NULL; > > g_free(chr->filename); > > + chr->filename = NULL; > > + s->connected = 0; > > +} > > + > > +static void tcp_chr_disconnect(CharDriverState *chr) > > +{ > > + TCPCharDriver *s = chr->opaque; > > + > > + if (!s->connected) { > > + return; > > + } > > + > > + tcp_chr_free_connection(chr); > > + > > + if (s->listen_ioc) { > > + s->listen_tag = qio_channel_add_watch( > > + QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, > > NULL); > > + } > > I think > > if (s->read_msgfds_num) { > for (i = 0; i < s->read_msgfds_num; i++) { > close(s->read_msgfds[i]); > } > g_free(s->read_msgfds); > } > > > should be moved from tcp_chr_close to tcp_chr_free_connection. The > following parts of tcp_chr_close instead are now duplicate, and can be > removed: > > remove_fd_in_watch(chr); > if (s->ioc) { > object_unref(OBJECT(s->ioc)); > } > if (s->write_msgfds_num) { > g_free(s->write_msgfds); > } > > are now duplicate and can be removed. correct > > Finally, since you're at it, here in tcp_set_msgfds: > > /* clear old pending fd array */ > g_free(s->write_msgfds); > s->write_msgfds = NULL; > > it's best to add a s->write_msgfds_num = 0. That makes sense, for the error case. done