Patchwork [3/5] libcacard/vscclient: fix leakage of socket on error paths

login
register
mail settings
Submitter Alon Levy
Date June 4, 2013, 8:23 p.m.
Message ID <1370377419-31788-3-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/248855/
State New
Headers show

Comments

Alon Levy - June 4, 2013, 8:23 p.m.
Spotted by Coverity.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 libcacard/vscclient.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Peter Maydell - June 4, 2013, 8:55 p.m.
On 4 June 2013 21:23, Alon Levy <alevy@redhat.com> wrote:
> Spotted by Coverity.
>
> Signed-off-by: Alon Levy <alevy@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Michael Tokarev - June 12, 2013, 12:09 p.m.
05.06.2013 00:23, Alon Levy wrote:
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -759,5 +763,6 @@ main(
>      g_io_channel_unref(channel_socket);
>      g_byte_array_unref(socket_to_send);
>  
> +    closesocket(sock);
>      return 0;
>  }

This one isn't really needed, -- there's no need to close
filedescriptors at the end of main().  I understand the
memory unref/free calls above it, to make valgrind and
glib trackers happy.  But it ofcourse does not hurt.

Besides, in all these error places it'd be really nice
to print the actual cause of the problem too - like
strerror(errno) or something like that.  Unfortunately we
had too many of these already in all parts of the code,
and it really is sometimes difficult to understand WHY
it fails without seeing the actual cause.  I'll send a
separate patch for that.

Thanks, applied to the trivial patches queue.

/mjt

Patch

diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index ac23647..5180d29 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -618,18 +618,22 @@  connect_to_qemu(
     if (ret != 0) {
         /* Error */
         fprintf(stderr, "getaddrinfo failed\n");
-        return -1;
+        goto cleanup_socket;
     }
 
     if (connect(sock, server->ai_addr, server->ai_addrlen) < 0) {
         /* Error */
         fprintf(stderr, "Could not connect\n");
-        return -1;
+        goto cleanup_socket;
     }
     if (verbose) {
         printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
     }
     return sock;
+
+cleanup_socket:
+    closesocket(sock);
+    return -1;
 }
 
 int
@@ -759,5 +763,6 @@  main(
     g_io_channel_unref(channel_socket);
     g_byte_array_unref(socket_to_send);
 
+    closesocket(sock);
     return 0;
 }