[Qemu-devel] [PATCH for-2.7 1/2] net: fix socket connect

[Marc-André Lureau]

Commit 7e8449594c929 changed net socket code to use socket_*() functions
from include/qemu/sockets.h. However, the change broke the connection
part, because socket_connect() returns a connected socket that is
being ignored. Use socket_connect() return value appropriately and fix
leaks on error path.

Note that after this change, the function is still blocking.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1368447

Signed-off-by: Marc-André Lureau <address@hidden>
---
 net/socket.c | 54 +++++++++++++++++++++++-------------------------------
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 17e635d..645bcb0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -501,6 +501,7 @@ static int net_socket_listen_init(NetClientState *peer,
 
     ret = socket_listen(saddr, &local_error);
     if (ret < 0) {
+        qapi_free_SocketAddress(saddr);
         error_report_err(local_error);
         return -1;
     }
@@ -522,9 +523,9 @@ static int net_socket_connect_init(NetClientState *peer,
                                    const char *host_str)
 {
     NetSocketState *s;
-    int fd, connected, ret;
-    char *addr_str;
-    SocketAddress *saddr;
+    int fd = -1, ret = -1;
+    char *addr_str = NULL;
+    SocketAddress *saddr = NULL;
     Error *local_error = NULL;
 
     saddr = socket_parse(host_str, &local_error);
@@ -533,45 +534,36 @@ static int net_socket_connect_init(NetClientState *peer,
         return -1;
     }
 
-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    fd = socket_connect(saddr, &local_error, NULL, NULL);
     if (fd < 0) {
-        perror("socket");
-        return -1;
+        error_report_err(local_error);
+        goto end;
     }
+
     qemu_set_nonblock(fd);
-    connected = 0;
-    for(;;) {
-        ret = socket_connect(saddr, &local_error, NULL, NULL);
-        if (ret < 0) {
-            if (errno == EINTR || errno == EWOULDBLOCK) {
-                /* continue */
-            } else if (errno == EINPROGRESS ||
-                       errno == EALREADY ||
-                       errno == EINVAL) {
-                break;
-            } else {
-                error_report_err(local_error);
-                closesocket(fd);
-                return -1;
-            }
-        } else {
-            connected = 1;
-            break;
-        }
+
+    s = net_socket_fd_init(peer, model, name, fd, true);
+    if (!s) {
+        goto end;
     }
-    s = net_socket_fd_init(peer, model, name, fd, connected);
-    if (!s)
-        return -1;
 
     addr_str = socket_address_to_string(saddr, &local_error);
-    if (addr_str == NULL)
-        return -1;
+    if (addr_str == NULL) {
+        error_report_err(local_error);
+        goto end;
+    }
 
     snprintf(s->nc.info_str, sizeof(s->nc.info_str),
              "socket: connect to %s", addr_str);
+    ret = 0;
+
+end:
+    if (ret == -1 && fd >= 0) {
+        closesocket(fd);
+    }
     qapi_free_SocketAddress(saddr);
     g_free(addr_str);
-    return 0;
+    return ret;
 }
 
 static int net_socket_mcast_init(NetClientState *peer,
-- 
2.9.0