Re: [Qemu-devel] [PATCH] Modify net/socket.c to use socket_* functions from include/qemu/sockets.h

[Ashi]

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 <address@hidden>
> > ---
> >   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