qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] libcacard: fix resource leak


From: zhanghailiang
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] libcacard: fix resource leak
Date: Mon, 17 Nov 2014 13:18:13 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Thunderbird/31.1.1

On 2014/11/14 17:29, Markus Armbruster wrote:
zhanghailiang <address@hidden> writes:

In function connect_to_qemu(), getaddrinfo() will allocate memory
that is stored into server, it should be freed by using freeaddrinfo()
before connect_to_qemu() return.

Signed-off-by: zhanghailiang <address@hidden>
---
v2:
- fix typo in title ;)
---
  libcacard/vscclient.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 80111df..fa6041d 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -597,7 +597,7 @@ connect_to_qemu(
      const char *port
  ) {
      struct addrinfo hints;
-    struct addrinfo *server;
+    struct addrinfo *server = NULL;
      int ret, sock;

      sock = socket(AF_INET, SOCK_STREAM, 0);
@@ -629,9 +629,14 @@ connect_to_qemu(
      if (verbose) {
          printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
      }
+
+    freeaddrinfo(server);
      return sock;

  cleanup_socket:
+    if (server) {
+        freeaddrinfo(server);
+    }
      closesocket(sock);
      return -1;
  }

Reviewed-by: Markus Armbruster <address@hidden>

Aside: this code uses the first result from getaddrinfo(), and fails if
it can't connect.  This is a common misuse of getaddrinfo().

Consider a remote host with both an IPv4 and an IPv6 address, but only
one of them actually connects.  If getaddrinfo() puts the connectable
one first, we succeed.  Else, we fail.

We should try the addresses in order until connect() succeeds, like
qemu-sockets.c does.

Good catch, i will fix it soon. And this patch mainly fix the coverity warning.
Maybe i should fix the problem after this patch been merged. ;) Thanks.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]