[virt-tools-list] [libosinfo 07/11] OsinfoInstallScript: Use an OsinfoInstallConfigParamList

[Christophe Fergeau]

On Tue, Dec 11, 2012 at 04:11:53PM +0200, Zeeshan Ali (Khattak) wrote:
> On Tue, Dec 11, 2012 at 11:52 AM, Christophe Fergeau
> <cfergeau at redhat.com> 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 at redhat.com> 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.

Yep, I can add a comment. Not sure where to mention that in the
documentation as IDs don't appear anywhere in install script parameters.
To be honest, this API would fit better in OsinfoInstallScriptParamsList.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20121211/0e1d6f69/attachment.sig>