[virt-tools-list] [PATCH 1/3] Change install script output (tool)

[Zeeshan Ali (Khattak)]

Hi,

On Thu, Aug 9, 2012 at 4:18 PM, Zeeshan Ali (Khattak)
<zeeshanak at gnome.org> wrote:
> On Thu, Aug 9, 2012 at 6:14 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>> On Wed, Aug 8, 2012 at 2:39 PM, Zeeshan Ali (Khattak)
>> <zeeshanak at gnome.org> wrote:
>>> On Thu, Aug 2, 2012 at 7:39 PM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
>>>
>>>> +    GSimpleAsyncResult *res;
>>>> +    GCancellable *cancellable;
>>>> +    GError *error;
>>>> +    GFile *file;
>>>> +    GFileOutputStream *stream;
>>>> +    gchar *output;
>>>> +    gssize output_len;
>>>> +    gssize output_pos;
>>>> +};
>>>> +
>>>> +static void osinfo_install_script_generate_output_free(struct OsinfoInstallScriptGenerateOutput *data)
>>>> +{
>>>> +    g_object_unref(data->stream);
>>>> +    g_object_unref(data->res);
>>>> +    g_free(data);
>>>> +}
>>>> +
>>>>
>>>>  static xsltStylesheetPtr osinfo_install_script_load_template(const gchar *uri,
>>>>                                                               const gchar *template,
>>>> @@ -621,9 +690,9 @@ void osinfo_install_script_generate_async(OsinfoInstallScript *script,
>>>>      }
>>>>  }
>>>>
>>>> -gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>>> -                                             GAsyncResult *res,
>>>> -                                             GError **error)
>>>> +static gpointer osinfo_install_script_generate_finish_common(OsinfoInstallScript *script,
>>>> +                                                             GAsyncResult *res,
>>>> +                                                             GError **error)
>>>>  {
>>>>      GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
>>>>
>>>> @@ -635,13 +704,61 @@ gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>>>      return g_simple_async_result_get_op_res_gpointer(simple);
>>>>  }
>>>>
>>>> +gchar *osinfo_install_script_generate_finish(OsinfoInstallScript *script,
>>>> +                                             GAsyncResult *res,
>>>> +                                             GError **error)
>>>> +{
>>>> +    return osinfo_install_script_generate_finish_common(script,
>>>> +                                                        res,
>>>> +                                                        error);
>>>> +}
>>>> +
>>>> +GFile *osinfo_install_script_generate_output_finish(OsinfoInstallScript *script,
>>>> +                                                    GAsyncResult *res,
>>>> +                                                    GError **error)
>>>
>>> So the function with output suffix gives you a GFile while the one w/o
>>> it gives you the path? I'd suggest only having one function named
>>> 'osinfo_install_script_generate' that returns the GFile. Its very easy
>>> to get name from GFile if app needs that.
>>
>> Sorry, probably I'm missing something.
>> But we are, in the both cases, returning a GFile, no?
>
> Huh? You are returning a string from osinfo_install_script_generate()
> and osinfo_install_script_generate_finish().

Sorry, it was me who was missing/forgetting the fact that
osinfo_install_script_generate() does not return a filename/path but
the script itself. So these functions remain as is.

With this in mind, I think you can simplify the code a bit by having
the commong functionality in osinfo_install_script_generate*() so
there is no need for another set of _common functions.


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124