On Tue, Dec 11, 2012 at 11:52 AM, Christophe Fergeau
<cfergeau@xxxxxxxxxx> wrote:
> On Tue, Dec 11, 2012 at 12:20:09AM +0200, Zeeshan Ali (Khattak) wrote:
>> On Mon, Dec 10, 2012 at 6:46 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
>> > gboolean osinfo_install_script_has_config_param_name(const OsinfoInstallScript *script, const gchar *name)
>> > {
>> > - GList *l;
>> > -
>> > - for (l = script->priv->config_param_list; l != NULL; l = l->next) {
>> > - OsinfoInstallConfigParam *tmp = l->data;
>> > -
>> > - if (g_strcmp0(osinfo_install_config_param_get_name(tmp), name) == 0)
>> > - return TRUE;
>> > - }
>> > - return FALSE;
>> > + OsinfoList *l = OSINFO_LIST(script->priv->config_params);
>> > + return (osinfo_list_find_by_id(l, name) != NULL);
>>
>> Should this code be assuming that name and ID are the same thing for
>> ConfigParam?
>
> This is discussed a bit in 'Set id in osinfo_install_config_param_new'
> commit log. OsinfoInstallConfigParam should have had an id from the start
> as it's an entity, but it currently does not. The patch mentioned above
> uses the name as id.
> I'm fine with changing the code from these various functions back to list
> iterations so that we don't rely on 'id' and 'name' being the same, but I
> hate code doing lookup in lists ;)
You are not alone. :) I was only thinking aloud. Do as you think is
best but if you go with this approach, perhaps adding this fact to
OsinfoInstallConfigParam documentation might be a good idea (if you
haven't done that already) and a comment in here as well.
--
Regards,
Zeeshan Ali (Khattak)
FSF member#5124
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
