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

[Markus Armbruster]

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.