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

[Fabiano Fidêncio]

On Mon, Oct 1, 2012 at 10:10 AM, Zeeshan Ali (Khattak)
<zeeshanak at gnome.org> wrote:
> 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.

Oh. You're right!
I'll fix this and do explicit check for NULL (as suggested by Zesshan
on IRC) and resend with the Avatar Info stuff.
Thank you.

>
> --
> Regards,
>
> Zeeshan Ali (Khattak)
> FSF member#5124


Best Regards,
-- 
Fabiano Fidêncio