[PATCH v2] block/curl: factor common cleanup and free password

zhaoguohan@kylinos.cn posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260324061345.975935-1-zhaoguohan@kylinos.cn
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
block/curl.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
[PATCH v2] block/curl: factor common cleanup and free password
Posted by zhaoguohan@kylinos.cn 1 week, 3 days ago
From: GuoHan Zhao <zhaoguohan@kylinos.cn>

Factor duplicated cleanup code into a helper and reuse it in the
open failure path and curl_close().

Also free s->password, fixing a leak.

Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
 block/curl.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 66aecfb20ec6..577b8d5a4e67 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -622,6 +622,27 @@ static void curl_attach_aio_context(BlockDriverState *bs,
     curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
 }
 
+static void curl_cleanup(BDRVCURLState *s)
+{
+    g_free(s->cookie);
+    s->cookie = NULL;
+    g_free(s->url);
+    s->url = NULL;
+    g_free(s->username);
+    s->username = NULL;
+    g_free(s->password);
+    s->password = NULL;
+    g_free(s->proxyusername);
+    s->proxyusername = NULL;
+    g_free(s->proxypassword);
+    s->proxypassword = NULL;
+    if (s->sockets) {
+        curl_drop_all_sockets(s->sockets);
+        g_hash_table_destroy(s->sockets);
+        s->sockets = NULL;
+    }
+}
+
 static QemuOptsList runtime_opts = {
     .name = "curl",
     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
@@ -900,15 +921,7 @@ out:
     state->curl = NULL;
 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);
-    if (s->sockets) {
-        curl_drop_all_sockets(s->sockets);
-        g_hash_table_destroy(s->sockets);
-    }
+    curl_cleanup(s);
     qemu_opts_del(opts);
     return -EINVAL;
 }
@@ -1010,12 +1023,7 @@ static void curl_close(BlockDriverState *bs)
     curl_detach_aio_context(bs);
     qemu_mutex_destroy(&s->mutex);
 
-    g_hash_table_destroy(s->sockets);
-    g_free(s->cookie);
-    g_free(s->url);
-    g_free(s->username);
-    g_free(s->proxyusername);
-    g_free(s->proxypassword);
+    curl_cleanup(s);
 }
 
 static int64_t coroutine_fn curl_co_getlength(BlockDriverState *bs)
-- 
2.43.0
Re: [PATCH v2] block/curl: factor common cleanup and free password
Posted by Daniel P. Berrangé 1 week, 3 days ago
On Tue, Mar 24, 2026 at 02:13:45PM +0800, zhaoguohan@kylinos.cn wrote:
> From: GuoHan Zhao <zhaoguohan@kylinos.cn>
> 
> Factor duplicated cleanup code into a helper and reuse it in the
> open failure path and curl_close().
> 
> Also free s->password, fixing a leak.

Please don't mix bug fixes with refactorings. These should be
separate commits.

> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
> ---
>  block/curl.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 66aecfb20ec6..577b8d5a4e67 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -622,6 +622,27 @@ static void curl_attach_aio_context(BlockDriverState *bs,
>      curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
>  }
>  
> +static void curl_cleanup(BDRVCURLState *s)
> +{
> +    g_free(s->cookie);
> +    s->cookie = NULL;

Use this pattern:

  g_clear_pointer(&s->cookie, g_free);

> +    g_free(s->url);
> +    s->url = NULL;
> +    g_free(s->username);
> +    s->username = NULL;
> +    g_free(s->password);
> +    s->password = NULL;
> +    g_free(s->proxyusername);
> +    s->proxyusername = NULL;
> +    g_free(s->proxypassword);
> +    s->proxypassword = NULL;
> +    if (s->sockets) {
> +        curl_drop_all_sockets(s->sockets);
> +        g_hash_table_destroy(s->sockets);
> +        s->sockets = NULL;

  g_clear_pointer(&s->sockets, g_hash_table_destroy)

> +    }
> +}
> +
>  static QemuOptsList runtime_opts = {
>      .name = "curl",
>      .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> @@ -900,15 +921,7 @@ out:
>      state->curl = NULL;
>  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);
> -    if (s->sockets) {
> -        curl_drop_all_sockets(s->sockets);
> -        g_hash_table_destroy(s->sockets);
> -    }
> +    curl_cleanup(s);
>      qemu_opts_del(opts);
>      return -EINVAL;
>  }
> @@ -1010,12 +1023,7 @@ static void curl_close(BlockDriverState *bs)
>      curl_detach_aio_context(bs);
>      qemu_mutex_destroy(&s->mutex);
>  
> -    g_hash_table_destroy(s->sockets);
> -    g_free(s->cookie);
> -    g_free(s->url);
> -    g_free(s->username);
> -    g_free(s->proxyusername);
> -    g_free(s->proxypassword);
> +    curl_cleanup(s);
>  }
>  
>  static int64_t coroutine_fn curl_co_getlength(BlockDriverState *bs)
> -- 
> 2.43.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|