From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7gQt-0003ra-Ca for qemu-devel@nongnu.org; Tue, 31 May 2016 05:57:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7gQo-0005vk-KC for qemu-devel@nongnu.org; Tue, 31 May 2016 05:57:55 -0400 Received: from mail-pa0-x242.google.com ([2607:f8b0:400e:c03::242]:33047) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7gQo-0005ug-D8 for qemu-devel@nongnu.org; Tue, 31 May 2016 05:57:50 -0400 Received: by mail-pa0-x242.google.com with SMTP id f8so24558433pag.0 for ; Tue, 31 May 2016 02:57:50 -0700 (PDT) References: <1463072585-10566-1-git-send-email-ashijeetacharya@gmail.com> <20160516164123.GC15256@stefanha-x1.localdomain> From: Ashi Message-ID: <574D6016.8020706@gmail.com> Date: Tue, 31 May 2016 15:27:42 +0530 MIME-Version: 1.0 In-Reply-To: <20160516164123.GC15256@stefanha-x1.localdomain> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Modify net/socket.c to use socket_* functions from include/qemu/sockets.h List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: jasowang@redhat.com, qemu-devel@nongnu.org On Monday 16 May 2016 10:11 PM, Stefan Hajnoczi wrote: > On Thu, May 12, 2016 at 10:33:05PM +0530, Ashijeet Acharya wrote: >> Changed the listen(),connect(),parse_host_port() in net/socket.c with the socket_*()functions in include/qemu/sockets.h. > > What is the rationale for this change? Please explain why this is > necessary or a good idea. This patch consists of basic api conversion since i guess everything will be using QAPI based socket_* functions in the future and the same task was listed on this http://wiki.qemu.org/BiteSizedTasks page too. > > Please summarize the address syntax changes in this patch and update the > QEMU man page. Syntax changes: 1. connect() -> socket_connect() 2. listen() -> socket_listen() 3. parse_host_port() -> socket_parse() 4, delete bind as it is automatically done inside socket_listen. 5. use SocketAddress as data-type of socket variable saddr. > >> >> Signed-off-by: Ashijeet Acharya >> --- >> net/socket.c | 38 +++++++++++++++++++------------------- >> 1 file changed, 19 insertions(+), 19 deletions(-) >> >> diff --git a/net/socket.c b/net/socket.c >> index 9fa2cd8..b6e2f3e 100644 >> --- a/net/socket.c >> +++ b/net/socket.c >> @@ -522,10 +522,12 @@ static int net_socket_listen_init(NetClientState *peer, >> { >> NetClientState *nc; >> NetSocketState *s; >> - struct sockaddr_in saddr; >> + SocketAddress *saddr; >> int fd, ret; >> + Error *local_error = NULL; >> + saddr = g_new0(SocketAddress, 1); >> >> - if (parse_host_port(&saddr, host_str) < 0) >> + if (socket_parse(host_str, &local_error) < 0) >> return -1; > > saddr is leaked. Please check all return code paths and avoid memory > leaks. > > I'm not sure if it makes sense to allocate a new SocketAddress object > since it is assigned a different object further down in the patch: > > saddr = socket_local_address(fd, &local_error); > I have looked into it and hopefully solved the memory leakage problem as you can see in the new patch. Thanks Ashijeet Acharya