On Mon, Apr 16, 2012 at 02:17:49PM -0700, Ethan Jackson wrote:
> If socket-util failed to modify the dscp bits of an active
> connection, it would fail to close the file descriptor potentially
> causing a leak. Found by inspection.
> 
> Signed-off-by: Ethan Jackson <et...@nicira.com>

I agree that this fixes a problem, but the exit path in this function
is hard to understand.  How about this instead:

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 6554e97..eb9ce7a 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -571,7 +571,7 @@ inet_open_active(int style, const char *target, uint16_t 
default_port,
     }
     error = set_nonblocking(fd);
     if (error) {
-        goto exit_close;
+        goto exit;
     }
 
     /* The socket options set here ensure that the TOS bits are set during
@@ -589,15 +589,8 @@ inet_open_active(int style, const char *target, uint16_t 
default_port,
     error = connect(fd, (struct sockaddr *) &sin, sizeof sin) == 0 ? 0 : errno;
     if (error == EINPROGRESS) {
         error = EAGAIN;
-    } else if (error && error != EAGAIN) {
-        goto exit_close;
     }
 
-    /* Success: error is 0 or EAGAIN. */
-    goto exit;
-
-exit_close:
-    close(fd);
 exit:
     if (!error || error == EAGAIN) {
         if (sinp) {
@@ -605,6 +598,9 @@ exit:
         }
         *fdp = fd;
     } else {
+        if (fd != -1) {
+            close(fd);
+        }
         *fdp = -1;
     }
     return error;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to