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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|