[3/3] net: complete NetSocketS?tate lifecycle handling

From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

The NetSocketState struct contains two file descriptors: an active
connection and a listen socket for new connections.  It's important that
we clean up after ourselves so these file descriptors are initialized to
-1 when unused.  This allows makes it possible to call cleanup functions
only when the file descriptors are valid (not -1).

The specific issue solved by this patch is that we avoid calling
close(-1), close(0), and qemu_set_fd_handler(-1, net_socket_accept,
NULL, s).  All of these are either problematic or unclean (e.g. reported
as warnings by Valgrind).

Also stay consistent by bringing the link down when the active
connection is closed.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 net/socket.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

please ignore this. thanks.

On Tue, Jun 19, 2012 at 1:13 AM,  <zwu.kernel@gmail.com> wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> The NetSocketState struct contains two file descriptors: an active
> connection and a listen socket for new connections.  It's important that
> we clean up after ourselves so these file descriptors are initialized to
> -1 when unused.  This allows makes it possible to call cleanup functions
> only when the file descriptors are valid (not -1).
>
> The specific issue solved by this patch is that we avoid calling
> close(-1), close(0), and qemu_set_fd_handler(-1, net_socket_accept,
> NULL, s).  All of these are either problematic or unclean (e.g. reported
> as warnings by Valgrind).
>
> Also stay consistent by bringing the link down when the active
> connection is closed.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  net/socket.c |   24 +++++++++++++++++++-----
>  1 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index 408d00e..9b15479 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -82,13 +82,16 @@ static void net_socket_send(void *opaque)
>         /* end of connection */
>     eoc:
>         qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> -        qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
> +        if (s->listen_fd != -1) {
> +            qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
> +        }
>         closesocket(s->fd);
>
>         s->fd = -1;
>         s->state = 0;
>         s->index = 0;
>         s->packet_len = 0;
> +        s->nc.link_down = true;
>         memset(s->buf, 0, sizeof(s->buf));
>         memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>
> @@ -239,8 +242,16 @@ fail:
>  static void net_socket_cleanup(NetClientState *nc)
>  {
>     NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
> -    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> -    close(s->fd);
> +    if (s->fd != -1) {
> +        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> +        close(s->fd);
> +        s->fd = -1;
> +    }
> +    if (s->listen_fd != -1) {
> +        qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
> +        closesocket(s->listen_fd);
> +        s->listen_fd = -1;
> +    }
>  }
>
>  static NetClientInfo net_dgram_socket_info = {
> @@ -302,6 +313,7 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
>     s = DO_UPCAST(NetSocketState, nc, nc);
>
>     s->fd = fd;
> +    s->listen_fd = -1;
>
>     qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s);
>
> @@ -345,6 +357,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>     s = DO_UPCAST(NetSocketState, nc, nc);
>
>     s->fd = fd;
> +    s->listen_fd = -1;
>
>     if (is_connected) {
>         net_socket_connect(s);
> @@ -411,7 +424,7 @@ static int net_socket_listen_init(NetClientState *peer,
>                                   const char *name,
>                                   const char *host_str)
>  {
> -    VLANClientState *nc;
> +    NetClientState *nc;
>     NetSocketState *s;
>     struct sockaddr_in saddr;
>     int fd, val, ret;
> @@ -443,8 +456,9 @@ static int net_socket_listen_init(NetClientState *peer,
>         return -1;
>     }
>
> -    nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name);
> +    nc = qemu_new_net_client(&net_socket_info, peer, model, name);
>     s = DO_UPCAST(NetSocketState, nc, nc);
> +    s->fd = -1;
>     s->listen_fd = fd;
>     s->nc.link_down = true;
>
> --
> 1.7.6
>
>