[virt-tools-list] [PATCH 1/3] differenciate between expected/output script name

[Zeeshan Ali (Khattak)]

On Mon, Oct 1, 2012 at 3:46 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
> We need to differenciate between the expected filename and the output
> filename. While former always remains the same (as some operating
> systems expect it with a particular name), the latter is dependent on
> the output prefix (set by application)

Looks good otherwise. One thing:

> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
> index bb2c2eb..8efe5f1 100644
> --- a/osinfo/osinfo_install_script.c
> +++ b/osinfo/osinfo_install_script.c
> @@ -353,10 +353,41 @@ const gchar *osinfo_install_script_get_output_prefix(OsinfoInstallScript *script
>      return script->priv->output_prefix;
>  }
>
> -const gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *script)
> +/**
> + * osinfo_install_script_get_expected_filename:
> + *
> + * Some operating systems (as Windows) expect that script filename has
> + * particular name to work.
> + *
> + * Returns: (transfer none): the expected script filename
> + */
> +const gchar *osinfo_install_script_get_expected_filename(OsinfoInstallScript *script)
>  {
>      return osinfo_entity_get_param_value(OSINFO_ENTITY(script),
> -                                         OSINFO_INSTALL_SCRIPT_PROP_OUTPUT_FILENAME);
> +                                         OSINFO_INSTALL_SCRIPT_PROP_FILENAME);
> +}
> +
> +/**
> + * osinfo_install_script_get_output_filename:
> + *
> + * Some operating systems are able to use any script filename, allowing the
> + * application to set the filename as desired. libosinfo provides this
> + * functionality by set the expected filename's prefix using
> + * osinfo_install_script_set_output_prefix() function.
> + *
> + * Returns: (transfer full): the output script filename
> + */
> +gchar *osinfo_install_script_get_output_filename(OsinfoInstallScript *script)
> +{
> +    char *output_filename = osinfo_install_script_get_expected_filename(script);
> +
> +    if (script->priv->output_prefix)
> +        output_filename = g_strjoin("-",
> +                                    script->priv->output_prefix,
> +                                    output_filename,
> +                                    NULL);
> +
> +    return output_filename;
>  }

If output_prefix is NULL, you are returning a const string (You must
have gotten some warnings from gcc about this?) while this function is
declaring to return a dynamically allocated one. It would be nice to
keep this functions' return consistent with get_expected_filename() so
I suggestion you keep an internal output_filename that you update
whenever output_prefix is changed.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124