[Libosinfo] [PATCHv4 07/11] OsinfoInstallConfig: Use config-params if set

[Zeeshan Ali (Khattak)]

On Thu, Dec 20, 2012 at 6:45 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> Now that OsinfoInstallConfig has a 'config-params' property
> which describes the config parameters when it's set, we can
> use it when it's available. OsinfoInstallConfigParams can indeed
> contain a datamap to be used to translate generic libosinfo values
> to OS-specific values.
> This commit introduces an osinfo_install_config_get_param_value_list
> method that will be used in subsequent commits to get these
> OS-specific values when generating install scripts.
> ---
>  osinfo/osinfo_install_config.c         | 84 ++++++++++++++++++++++++++++++++++
>  osinfo/osinfo_install_config_private.h |  1 +
>  2 files changed, 85 insertions(+)
>
> diff --git a/osinfo/osinfo_install_config.c b/osinfo/osinfo_install_config.c
> index a77317b..4c42746 100644
> --- a/osinfo/osinfo_install_config.c
> +++ b/osinfo/osinfo_install_config.c
> @@ -737,6 +737,90 @@ OsinfoInstallConfigParamList *osinfo_install_config_get_config_params(OsinfoInst
>      return config->priv->config_params;
>  }
>
> +
> +static const gchar *
> +osinfo_install_config_transform_value(OsinfoInstallConfig *config,
> +                                      const gchar *key,
> +                                      const gchar *value)
> +{
> +    OsinfoDatamap *map;
> +    OsinfoEntity *entity;
> +    OsinfoInstallConfigParam *param;
> +    const gchar *transformed_value;
> +
> +    if (!config->priv->config_params)
> +        return value;
> +
> +    entity = osinfo_list_find_by_id(OSINFO_LIST(config->priv->config_params),
> +                                    key);
> +    if (entity == NULL) {
> +        g_warning("%s is not a known parameter for this config", key);
> +        return value;
> +    }
> +
> +    param = OSINFO_INSTALL_CONFIG_PARAM(entity);;
> +    map = osinfo_install_config_param_get_value_map(param);
> +    if (map == NULL) {
> +        g_debug("no remapping to be done for %s", key);
> +        return value;
> +    }
> +    transformed_value = osinfo_datamap_lookup(map, value);
> +    if (transformed_value == NULL) {
> +        g_warning("value not present in %s datamap: %s", key, value);
> +        return value;
> +    }
> +
> +    return transformed_value;
> +}
> +
> +static GHashTable *get_remapped_keys_once(void)

Two questions:

1. Do we really need this hashtable here and hardcode the params that
will be transformed? It seems to me that
osinfo_install_config_transform_value() can work just fine without it,
depending on whether or not the config param in question has a datamap
associated or not.

2. Wouldn't the code be more simpler with this hashtable
initialization in class_init (so you dont have to to use GOnce API)?

Both these apply to internal code only so answer to these questions
shouldn't block these patches to be pushed already.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124