diff mbox

[for-2.7,v3,04/36] qga: free remaining leaking state

Message ID 20160803145541.15355-5-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau Aug. 3, 2016, 2:55 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/guest-agent-command-state.c | 7 +++++++
 qga/guest-agent-core.h          | 1 +
 qga/main.c                      | 6 ++++++
 3 files changed, 14 insertions(+)

Comments

Markus Armbruster Aug. 4, 2016, 1:38 p.m. UTC | #1 
Copying the QGA maintainer.

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  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-state.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 = 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:
>      }
>  
>      config_free(config);
> +    if (s->main_loop) {
> +        g_main_loop_unref(s->main_loop);
> +    }
> +    g_free(s);
>  
>      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.
diff mbox

Patch

diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.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 = 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);
+    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:
     }
 
     config_free(config);
+    if (s->main_loop) {
+        g_main_loop_unref(s->main_loop);
+    }
+    g_free(s);
 
     return ret;
 }