Hey! On Thu, Nov 8, 2018 at 10:41 AM Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > 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 <fidencio@xxxxxxxxxx> > > --- > > 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? Just for the record, I've talked with Christophe on IRC ... Basically if we move script declaration out of the loop we'd have to set it to NULL each iteration. The new patch would look like this: --- a/osinfo/osinfo_os.c +++ b/osinfo/osinfo_os.c @@ -611,16 +611,20 @@ 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 *script = NULL; while (tmp) { - OsinfoInstallScript *script = tmp->data; + script = tmp->data; if (g_str_equal(profile, osinfo_install_script_get_profile(script))) - return script; + break; tmp = tmp->next; + script = NULL; } - return NULL; + g_list_free(scripts); + + return script; } -- We also agreed to go for this version (the new one). > > Looks good apart from this. > > Christophe > > > > > tmp = tmp->next; > > } > > > > - return NULL; > > + g_list_free(scripts); > > + > > + return s; > > } > > > > > > -- > > 2.19.1 > > > > _______________________________________________ > > Libosinfo mailing list > > Libosinfo@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/libosinfo _______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
