Re: [Qemu-devel] [PATCH v2 03/37] qga: free the whole blacklist

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On 07/28/2016 08:37 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Free the list, not just the elements.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/main.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/qga/main.c b/qga/main.c
> index 4c3b2c7..67be90b 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
>  #ifdef CONFIG_FSFREEZE
>      g_free(config->fsfreeze_hook);
>  #endif
> +    g_list_foreach(config->blacklist, (GFunc)g_free, NULL);

What an ugly cast - undefined behavior according to C. But it happens to
work on all ABI that we support (calling a unary function as cast to
appear as a binary function silently ignores the second argument), so I
could sort of live with it, if it made the patch arguably smaller due to
fewer lines of code than required for a type-correct solution.

> +    g_list_free(config->blacklist);
>      g_free(config);
>  }
>  
> @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
>      return EXIT_SUCCESS;
>  }
>  
> -static void free_blacklist_entry(gpointer entry, gpointer unused)
> -{
> -    g_free(entry);
> -}

On the other hand, we already had EXACTLY the type-correct forwarder
function in place (it just needs to be hoisted earlier before the point
where you use it).

So I would really prefer that v3 use free_blacklist_entry() rather than
(GFunc)g_free as the argument to g_list_foreach().

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]