[Qemu-devel] [PATCH 3/3] net: complete NetSocketState lifecycle handling

[zwu.kernel@gmail.com]

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