From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSszI-0005fA-IP for qemu-devel@nongnu.org; Thu, 28 Jul 2016 17:37:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSszF-0005Ng-13 for qemu-devel@nongnu.org; Thu, 28 Jul 2016 17:37:04 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:33234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSszE-0005NQ-Ns for qemu-devel@nongnu.org; Thu, 28 Jul 2016 17:37:00 -0400 Date: Thu, 28 Jul 2016 17:36:59 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <503530863.9448361.1469741819715.JavaMail.zimbra@redhat.com> In-Reply-To: <579A76C0.8080104@redhat.com> References: <20160728143808.13707-1-marcandre.lureau@redhat.com> <20160728143808.13707-4-marcandre.lureau@redhat.com> <579A76C0.8080104@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 03/37] qga: free the whole blacklist List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: marcandre lureau , qemu-devel@nongnu.org ----- Original Message ----- > On 07/28/2016 08:37 AM, marcandre.lureau@redhat.com wrote: > > From: Marc-Andr=C3=A9 Lureau > >=20 > > Free the list, not just the elements. > >=20 > > Signed-off-by: Marc-Andr=C3=A9 Lureau > > --- > > qga/main.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > >=20 > > 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); >=20 > 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. Note that's how glib implements g_slist_free_full() and others, I just used= the same common code/style >=20 > > + g_list_free(config->blacklist); > > g_free(config); > > } > > =20 > > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *confi= g) > > return EXIT_SUCCESS; > > } > > =20 > > -static void free_blacklist_entry(gpointer entry, gpointer unused) > > -{ > > - g_free(entry); > > -} >=20 > 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). >=20 > So I would really prefer that v3 use free_blacklist_entry() rather than > (GFunc)g_free as the argument to g_list_foreach(). ok ;)