diff mbox series

[v3,6/7] block/curl: fix minor memory leaks

Message ID facec289ff8a7ea66e420585ac87baf65b3f4cd5.1510093478.git.jcody@redhat.com
State New
Headers show
Series Code cleanup and minor fixes (for 2.11) | expand

Commit Message

Jeff Cody Nov. 7, 2017, 10:27 p.m. UTC
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/curl.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Blake Nov. 8, 2017, 12:13 a.m. UTC | #1
On 11/07/2017 04:27 PM, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/curl.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
>      qemu_mutex_destroy(&s->mutex);
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);
>      qemu_opts_del(opts);
>      return -EINVAL;
>  }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>  
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);
>  }
>  
>  static int64_t curl_getlength(BlockDriverState *bs)
>
Philippe Mathieu-Daudé Nov. 8, 2017, 5:08 a.m. UTC | #2
Hi Jeff,

On 11/07/2017 07:27 PM, Jeff Cody wrote:

"Missed in 1bff9606429."

> Signed-off-by: Jeff Cody <jcody@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  block/curl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
>      qemu_mutex_destroy(&s->mutex);
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);
>      qemu_opts_del(opts);
>      return -EINVAL;
>  }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>  
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);
>  }
>  
>  static int64_t curl_getlength(BlockDriverState *bs)
>
Richard W.M. Jones Nov. 8, 2017, 11:45 a.m. UTC | #3
On Tue, Nov 07, 2017 at 05:27:23PM -0500, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/curl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 00a9879..35cf417 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -857,6 +857,9 @@ out_noclean:
>      qemu_mutex_destroy(&s->mutex);
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);
>      qemu_opts_del(opts);
>      return -EINVAL;
>  }
> @@ -955,6 +958,9 @@ static void curl_close(BlockDriverState *bs)
>  
>      g_free(s->cookie);
>      g_free(s->url);
> +    g_free(s->username);
> +    g_free(s->proxyusername);
> +    g_free(s->proxypassword);

username & proxyusername are allocated with g_strdup and so
should obviously be freed.

proxypassword is returned by qcrypto_secret_lookup_as_utf8, and it's
not clear to me if we should free that or not.  However examining the
code of qcrypto_secret_lookup it looks as if this string is allocated
by g_new0, which would indicate that it should also be freed.

Therefore:

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Rich.
diff mbox series

Patch

diff --git a/block/curl.c b/block/curl.c
index 00a9879..35cf417 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -857,6 +857,9 @@  out_noclean:
     qemu_mutex_destroy(&s->mutex);
     g_free(s->cookie);
     g_free(s->url);
+    g_free(s->username);
+    g_free(s->proxyusername);
+    g_free(s->proxypassword);
     qemu_opts_del(opts);
     return -EINVAL;
 }
@@ -955,6 +958,9 @@  static void curl_close(BlockDriverState *bs)
 
     g_free(s->cookie);
     g_free(s->url);
+    g_free(s->username);
+    g_free(s->proxyusername);
+    g_free(s->proxypassword);
 }
 
 static int64_t curl_getlength(BlockDriverState *bs)