Re: [Libosinfo] [libosinfo PATCH] os: Don't leak scripts list

2018-11-08 Thread Christophe Fergeau
hey,

On Wed, Nov 07, 2018 at 09:54:15PM +0100, Fabiano Fidêncio wrote:
> osinfo_list_get_elements() calls g_hash_table_get_values() which returns
> a GList that must be freed after used.
> 
> For more info, please, refer to:
> https://developer.gnome.org/glib/unstable/glib-Hash-Tables.html#g-hash-table-get-values
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  osinfo/osinfo_os.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/osinfo/osinfo_os.c b/osinfo/osinfo_os.c
> index 4f74331..f89861b 100644
> --- a/osinfo/osinfo_os.c
> +++ b/osinfo/osinfo_os.c
> @@ -611,16 +611,21 @@ OsinfoInstallScript 
> *osinfo_os_find_install_script(OsinfoOs *os, const gchar *pr
>  g_return_val_if_fail(OSINFO_IS_OS(os), NULL);
>  GList *scripts = osinfo_list_get_elements(OSINFO_LIST(os));
>  GList *tmp = scripts;
> +OsinfoInstallScript *s = NULL;
>  
>  while (tmp) {
>  OsinfoInstallScript *script = tmp->data;
> -if (g_str_equal(profile, osinfo_install_script_get_profile(script)))
> -return script;
> +if (g_str_equal(profile, osinfo_install_script_get_profile(script))) 
> {
> +s = script;
> +break;
> +}

Do we really need both 's' and 'script'? I think you could move 'script'
declaration out of the while() block, and achieve the same result?

Looks good apart from this.

Christophe

>  
>  tmp = tmp->next;
>  }
>  
> -return NULL;
> +g_list_free(scripts);
> +
> +return s;
>  }
>  
>  
> -- 
> 2.19.1
> 
> ___
> Libosinfo mailing list
> Libosinfo@redhat.com
> https://www.redhat.com/mailman/listinfo/libosinfo


signature.asc
Description: PGP signature
___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo


[Libosinfo] [libosinfo PATCH] os: Don't leak scripts list

2018-11-07 Thread Fabiano Fidêncio
osinfo_list_get_elements() calls g_hash_table_get_values() which returns
a GList that must be freed after used.

For more info, please, refer to:
https://developer.gnome.org/glib/unstable/glib-Hash-Tables.html#g-hash-table-get-values

Signed-off-by: Fabiano Fidêncio 
---
 osinfo/osinfo_os.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/osinfo/osinfo_os.c b/osinfo/osinfo_os.c
index 4f74331..f89861b 100644
--- a/osinfo/osinfo_os.c
+++ b/osinfo/osinfo_os.c
@@ -611,16 +611,21 @@ OsinfoInstallScript 
*osinfo_os_find_install_script(OsinfoOs *os, const gchar *pr
 g_return_val_if_fail(OSINFO_IS_OS(os), NULL);
 GList *scripts = osinfo_list_get_elements(OSINFO_LIST(os));
 GList *tmp = scripts;
+OsinfoInstallScript *s = NULL;
 
 while (tmp) {
 OsinfoInstallScript *script = tmp->data;
-if (g_str_equal(profile, osinfo_install_script_get_profile(script)))
-return script;
+if (g_str_equal(profile, osinfo_install_script_get_profile(script))) {
+s = script;
+break;
+}
 
 tmp = tmp->next;
 }
 
-return NULL;
+g_list_free(scripts);
+
+return s;
 }
 
 
-- 
2.19.1

___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo