From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60179) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVbzu-0001EH-Ha for qemu-devel@nongnu.org; Fri, 05 Aug 2016 06:04:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bVbzq-0003mA-97 for qemu-devel@nongnu.org; Fri, 05 Aug 2016 06:04:57 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:35405) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVbzp-0003lv-VB for qemu-devel@nongnu.org; Fri, 05 Aug 2016 06:04:54 -0400 Received: by mail-wm0-x244.google.com with SMTP id i5so3078802wmg.2 for ; Fri, 05 Aug 2016 03:04:53 -0700 (PDT) Sender: Paolo Bonzini References: <20160805082421.21994-1-marcandre.lureau@redhat.com> <20160805082421.21994-20-marcandre.lureau@redhat.com> From: Paolo Bonzini Message-ID: <9c41b569-35d7-7ef8-c83f-3f423e9130ea@redhat.com> Date: Fri, 5 Aug 2016 12:04:48 +0200 MIME-Version: 1.0 In-Reply-To: <20160805082421.21994-20-marcandre.lureau@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH for-2.7 v4 19/36] char: free the tcp connection data when closing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: armbru@redhat.com On 05/08/2016 10:24, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau > > Make sure the connection data got freed when closing the chardev, to > avoid leaks. Introduce tcp_chr_free_connection() to clean all connection > related data, and move some tcp_chr_close() clean-ups there. > > (while at it, set write_msgfds_num to 0 when clearing array in > tcp_set_msgfds()) > > Signed-off-by: Marc-André Lureau > --- > qemu-char.c | 50 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 31 insertions(+), 19 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index a50b8fb..f20d385 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2763,6 +2763,7 @@ static int tcp_set_msgfds(CharDriverState *chr, int *fds, int num) > /* clear old pending fd array */ > g_free(s->write_msgfds); > s->write_msgfds = NULL; > + s->write_msgfds_num = 0; > > if (!s->connected || > !qio_channel_has_feature(s->ioc, > @@ -2843,19 +2844,24 @@ static GSource *tcp_chr_add_watch(CharDriverState *chr, GIOCondition cond) > return qio_channel_create_watch(s->ioc, cond); > } > > -static void tcp_chr_disconnect(CharDriverState *chr) > +static void tcp_chr_free_connection(CharDriverState *chr) > { > TCPCharDriver *s = chr->opaque; > + int i; > > if (!s->connected) { > 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); > + if (s->read_msgfds_num) { > + for (i = 0; i < s->read_msgfds_num; i++) { > + close(s->read_msgfds[i]); > + } > + g_free(s->read_msgfds); > + s->read_msgfds = NULL; > + s->read_msgfds_num = 0; > } > + > tcp_set_msgfds(chr, NULL, 0); > remove_fd_in_watch(chr); > object_unref(OBJECT(s->sioc)); > @@ -2863,6 +2869,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); > + } > chr->filename = SocketAddress_to_str("disconnected:", s->addr, > s->is_listen, s->is_telnet); > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > @@ -3177,17 +3201,14 @@ int qemu_chr_wait_connected(CharDriverState *chr, Error **errp) > static void tcp_chr_close(CharDriverState *chr) > { > TCPCharDriver *s = chr->opaque; > - int i; > + > + tcp_chr_free_connection(chr); > > if (s->reconnect_timer) { > g_source_remove(s->reconnect_timer); > s->reconnect_timer = 0; > } > qapi_free_SocketAddress(s->addr); > - remove_fd_in_watch(chr); > - if (s->ioc) { > - object_unref(OBJECT(s->ioc)); > - } > if (s->listen_tag) { > g_source_remove(s->listen_tag); > s->listen_tag = 0; > @@ -3195,18 +3216,9 @@ static void tcp_chr_close(CharDriverState *chr) > if (s->listen_ioc) { > object_unref(OBJECT(s->listen_ioc)); > } > - if (s->read_msgfds_num) { > - for (i = 0; i < s->read_msgfds_num; i++) { > - close(s->read_msgfds[i]); > - } > - g_free(s->read_msgfds); > - } > if (s->tls_creds) { > object_unref(OBJECT(s->tls_creds)); > } > - if (s->write_msgfds_num) { > - g_free(s->write_msgfds); > - } > g_free(s); > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > Reviewed-by: Paolo Bonzini