From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58890) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVIrA-00038Z-Ah for qemu-devel@nongnu.org; Thu, 04 Aug 2016 09:38:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bVIr5-00079B-DJ for qemu-devel@nongnu.org; Thu, 04 Aug 2016 09:38:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58088) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVIr5-00078n-4J for qemu-devel@nongnu.org; Thu, 04 Aug 2016 09:38:35 -0400 From: Markus Armbruster References: <20160803145541.15355-1-marcandre.lureau@redhat.com> <20160803145541.15355-5-marcandre.lureau@redhat.com> Date: Thu, 04 Aug 2016 15:38:32 +0200 In-Reply-To: <20160803145541.15355-5-marcandre.lureau@redhat.com> (marcandre lureau's message of "Wed, 3 Aug 2016 18:55:09 +0400") Message-ID: <87bn18fycn.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-2.7 v3 04/36] qga: free remaining leaking state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, Michael Roth Copying the QGA maintainer. marcandre.lureau@redhat.com writes: > From: Marc-Andr=C3=A9 Lureau > > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > qga/guest-agent-command-state.c | 7 +++++++ > qga/guest-agent-core.h | 1 + > qga/main.c | 6 ++++++ > 3 files changed, 14 insertions(+) > > diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-st= ate.c > index 4de229c..31c6368 100644 > --- a/qga/guest-agent-command-state.c > +++ b/qga/guest-agent-command-state.c > @@ -71,3 +71,10 @@ GACommandState *ga_command_state_new(void) > cs->groups =3D NULL; > return cs; > } > + > +void ga_command_state_free(GACommandState *cs) > +{ > + g_slist_foreach(cs->groups, qemu_g_func_free, NULL); > + g_slist_free(cs->groups); Remarks on g_list_free_full() in PATCH 03 apply to g_slist_free_full() here. > + g_free(cs); > +} > diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h > index 0a49516..63e9d39 100644 > --- a/qga/guest-agent-core.h > +++ b/qga/guest-agent-core.h > @@ -28,6 +28,7 @@ void ga_command_state_add(GACommandState *cs, > void ga_command_state_init_all(GACommandState *cs); > void ga_command_state_cleanup_all(GACommandState *cs); > GACommandState *ga_command_state_new(void); > +void ga_command_state_free(GACommandState *cs); > bool ga_logging_enabled(GAState *s); > void ga_disable_logging(GAState *s); > void ga_enable_logging(GAState *s); > diff --git a/qga/main.c b/qga/main.c > index 868508b..0038702 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -1372,6 +1372,8 @@ int main(int argc, char **argv) > end: > if (s->command_state) { > ga_command_state_cleanup_all(s->command_state); > + ga_command_state_free(s->command_state); > + json_message_parser_destroy(&s->parser); > } > if (s->channel) { > ga_channel_free(s->channel); > @@ -1384,6 +1386,10 @@ end: > } >=20=20 > config_free(config); > + if (s->main_loop) { > + g_main_loop_unref(s->main_loop); > + } > + g_free(s); >=20=20 > return ret; > } Not bothering to free memory immediately before terminating the process is not a leak. The commit message shouldn't claim it is. Personally, I wouldn't bother with freeing memory there. Applies to PATCH 03, too. But it looks like the maintainer likes having it done. If that's true, then doing it completely is probably better. Leaving actual review to the maintainer.