Re: [PATCH v3 1/5] Use GTask instead of GSimpleAsyncResult

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi fidencio,

Thanks for doing this. Just some nits about commit log:

On Mon, Jan 11, 2016 at 1:19 PM, Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> For using the new GTask API, let's bump GIO (part of GLib) dependency
> version to 2.36.

* I prefer to put version bump in separate patch, cause it kinda is a
separate change and it makes it hard to miss when writing release
notes.

* Empty lines before each paragraph please. Not really your fault.
Seems this very annoying habit is getting widespread. :(

> what is safe based on major distro support:

The last line doesn't make sense grammatically (only questions start
with 'what') and "safe" IMO is vague and incorrect here. Just say "All
major distros have 2.36 or higher version available:"

> - Debian Jessie: glib-2.42
> - RHEL-7.1: glib-2.40
> - SLES12: glib-2.38
> - Ubuntu LTS 14.04: glib-2.40
> ---
>  configure.ac                   |  2 +-
>  osinfo/osinfo_install_script.c | 52 ++++++++++++++----------------------
>  osinfo/osinfo_media.c          | 60 +++++++++++++++++-------------------------
>  osinfo/osinfo_tree.c           | 34 +++++++++---------------
>  4 files changed, 57 insertions(+), 91 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 4154134..5c38b96 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -37,7 +37,7 @@ m4_if(m4_version_compare([2.61a.100],
>  m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
>
>  PKG_CHECK_MODULES([GOBJECT], [gobject-2.0])
> -PKG_CHECK_MODULES([GIO], [gio-2.0])
> +PKG_CHECK_MODULES([GIO], [gio-2.0 >= 2.36])
>  PKG_CHECK_MODULES([SOUP], [libsoup-2.4 >= 2.42])
>  PKG_CHECK_MODULES([LIBXML], [libxml-2.0 >= 2.6.0])
>  PKG_CHECK_MODULES([LIBXSLT], [libxslt >= 1.0.0])
> diff --git a/osinfo/osinfo_install_script.c b/osinfo/osinfo_install_script.c
> index 9ded116..7f0d863 100644
> --- a/osinfo/osinfo_install_script.c
> +++ b/osinfo/osinfo_install_script.c
> @@ -531,7 +531,7 @@ OsinfoAvatarFormat *osinfo_install_script_get_avatar_format(OsinfoInstallScript
>  }
>
>  struct _OsinfoInstallScriptGenerateData {
> -    GSimpleAsyncResult *res;
> +    GTask *res;
>      OsinfoOs *os;
>      OsinfoMedia *media;
>      OsinfoInstallConfig *config;
> @@ -551,8 +551,7 @@ static void osinfo_install_script_generate_data_free(OsinfoInstallScriptGenerate
>  }
>
>  struct _OsinfoInstallScriptGenerateOutputData {
> -    GSimpleAsyncResult *res;
> -    GCancellable *cancellable;
> +    GTask *res;
>      GError *error;
>      GFile *file;
>      GFileOutputStream *stream;
> @@ -882,7 +881,7 @@ static void osinfo_install_script_template_loaded(GObject *src,
>                                       NULL,
>                                       &error)) {
>          g_prefix_error(&error, _("Failed to load script template %s: "), uri);
> -        g_simple_async_result_take_error(data->res, error);
> +        g_task_return_error(data->res, error);
>          goto cleanup;
>      }
>
> @@ -897,15 +896,13 @@ static void osinfo_install_script_template_loaded(GObject *src,
>                                                data->config,
>                                                &error)) {
>          g_prefix_error(&error, _("Failed to apply script template %s: "), uri);
> -        g_simple_async_result_take_error(data->res, error);
> +        g_task_return_error(data->res, error);
>          goto cleanup;
>      }
>
> -    g_simple_async_result_set_op_res_gpointer(data->res,
> -                                              output, NULL);
> +    g_task_return_pointer(data->res, output, NULL);
>
>   cleanup:
> -    g_simple_async_result_complete(data->res);
>      osinfo_install_script_generate_data_free(data);
>      g_free(uri);
>  }
> @@ -934,10 +931,10 @@ static void osinfo_install_script_generate_async_common(OsinfoInstallScript *scr
>          data->media = g_object_ref(media);
>      data->config = g_object_ref(config);
>      data->script = g_object_ref(script);
> -    data->res = g_simple_async_result_new(G_OBJECT(script),
> -                                          callback,
> -                                          user_data,
> -                                          osinfo_install_script_generate_async_common);
> +    data->res = g_task_new(G_OBJECT(script),
> +                           cancellable,
> +                           callback,
> +                           user_data);
>
>      if (templateData) {
>          GError *error = NULL;
> @@ -952,14 +949,11 @@ static void osinfo_install_script_generate_async_common(OsinfoInstallScript *scr
>                                                    data->config,
>                                                    &error)) {
>              g_prefix_error(&error, "%s", _("Failed to apply script template: "));
> -            g_simple_async_result_take_error(data->res, error);
> -            g_simple_async_result_complete(data->res);
> +            g_task_return_error(data->res, error);
>              osinfo_install_script_generate_data_free(data);
>              return;
>          }
> -        g_simple_async_result_set_op_res_gpointer(data->res,
> -                                                  output, NULL);
> -        g_simple_async_result_complete_in_idle(data->res);
> +        g_task_return_pointer(data->res, output, NULL);
>          osinfo_install_script_generate_data_free(data);
>      } else {
>          GFile *file = g_file_new_for_uri(templateUri);
> @@ -1007,14 +1001,11 @@ static gpointer osinfo_install_script_generate_finish_common(OsinfoInstallScript
>                                                               GAsyncResult *res,
>                                                               GError **error)
>  {
> -    GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
> +    GTask *task = G_TASK(res);
>
>      g_return_val_if_fail(error == NULL || *error == NULL, NULL);
>
> -    if (g_simple_async_result_propagate_error(simple, error))
> -        return NULL;
> -
> -    return g_simple_async_result_get_op_res_gpointer(simple);
> +    return g_task_propagate_pointer(task, error);
>  }
>
>  /**
> @@ -1132,9 +1123,7 @@ static void osinfo_install_script_generate_output_close_file(GObject *src,
>                                   res,
>                                   &data->error);
>
> -    g_simple_async_result_set_op_res_gpointer(data->res,
> -                                              data->file, NULL);
> -    g_simple_async_result_complete_in_idle(data->res);
> +    g_task_return_pointer(data->res, data->file, NULL);
>
>      osinfo_install_script_generate_output_data_free(data);
>  }
> @@ -1309,14 +1298,14 @@ static void osinfo_install_script_generate_output_write_file(GObject *src,
>                                      data->output + data->output_pos,
>                                      data->output_len - data->output_pos,
>                                      G_PRIORITY_DEFAULT,
> -                                    data->cancellable,
> +                                    g_task_get_cancellable(data->res),
>                                      osinfo_install_script_generate_output_write_file,
>                                      data);
>
>      } else {
>          g_output_stream_close_async(G_OUTPUT_STREAM(data->stream),
>                                      G_PRIORITY_DEFAULT,
> -                                    data->cancellable,
> +                                    g_task_get_cancellable(data->res),
>                                      osinfo_install_script_generate_output_close_file,
>                                      data);
>      }
> @@ -1338,12 +1327,11 @@ static void osinfo_install_script_generate_output_async_common(OsinfoInstallScri
>
>      OsinfoInstallScriptGenerateSyncData *data_sync = user_data;
>
> -    data->res = g_simple_async_result_new(G_OBJECT(script),
> -                                          callback,
> -                                          user_data,
> -                                          osinfo_install_script_generate_output_async_common);
> +    data->res = g_task_new(G_OBJECT(script),
> +                           cancellable,
> +                           callback,
> +                           user_data);
>
> -    data->cancellable = cancellable;
>      data->error = data_sync->error;
>      if (media != NULL) {
>          data->output = osinfo_install_script_generate_for_media(script,
> diff --git a/osinfo/osinfo_media.c b/osinfo/osinfo_media.c
> index 7ff48a9..d9fcba6 100644
> --- a/osinfo/osinfo_media.c
> +++ b/osinfo/osinfo_media.c
> @@ -75,10 +75,7 @@ typedef struct _CreateFromLocationAsyncData CreateFromLocationAsyncData;
>  struct _CreateFromLocationAsyncData {
>      GFile *file;
>
> -    gint priority;
> -    GCancellable *cancellable;
> -
> -    GSimpleAsyncResult *res;
> +    GTask *res;
>
>      PrimaryVolumeDescriptor pvd;
>      SupplementaryVolumeDescriptor svd;
> @@ -91,7 +88,6 @@ static void create_from_location_async_data_free
>                                  (CreateFromLocationAsyncData *data)
>  {
>     g_object_unref(data->file);
> -   g_clear_object(&data->cancellable);
>     g_object_unref(data->res);
>
>     g_slice_free(CreateFromLocationAsyncData, data);
> @@ -707,8 +703,8 @@ static void on_svd_read(GObject *source,
>          g_input_stream_read_async(stream,
>                                    ((gchar *)&data->svd + data->offset),
>                                    data->length - data->offset,
> -                                  data->priority,
> -                                  data->cancellable,
> +                                  g_task_get_priority(data->res),
> +                                  g_task_get_cancellable(data->res),
>                                    on_svd_read,
>                                    data);
>          return;
> @@ -760,10 +756,9 @@ static void on_svd_read(GObject *source,
>
>  EXIT:
>      if (error != NULL)
> -        g_simple_async_result_take_error(data->res, error);
> +        g_task_return_error(data->res, error);
>      else
> -        g_simple_async_result_set_op_res_gpointer(data->res, media, NULL);
> -    g_simple_async_result_complete(data->res);
> +        g_task_return_pointer(data->res, media, NULL);
>
>      g_object_unref(stream);
>      create_from_location_async_data_free(data);
> @@ -800,8 +795,8 @@ static void on_pvd_read(GObject *source,
>          g_input_stream_read_async(stream,
>                                    ((gchar*)&data->pvd) + data->offset,
>                                    data->length - data->offset,
> -                                  data->priority,
> -                                  data->cancellable,
> +                                  g_task_get_priority(data->res),
> +                                  g_task_get_cancellable(data->res),
>                                    on_pvd_read,
>                                    data);
>          return;
> @@ -827,15 +822,14 @@ static void on_pvd_read(GObject *source,
>      g_input_stream_read_async(stream,
>                                (gchar *)&data->svd,
>                                data->length,
> -                              data->priority,
> -                              data->cancellable,
> +                              g_task_get_priority(data->res),
> +                              g_task_get_cancellable(data->res),
>                                on_svd_read,
>                                data);
>      return;
>
>  ON_ERROR:
> -    g_simple_async_result_take_error(data->res, error);
> -    g_simple_async_result_complete(data->res);
> +    g_task_return_error(data->res, error);
>      create_from_location_async_data_free(data);
>  }
>
> @@ -857,8 +851,7 @@ static void on_location_skipped(GObject *source,
>                          OSINFO_MEDIA_ERROR,
>                          OSINFO_MEDIA_ERROR_NO_DESCRIPTORS,
>                          _("No volume descriptors"));
> -        g_simple_async_result_take_error(data->res, error);
> -        g_simple_async_result_complete(data->res);
> +        g_task_return_error(data->res, error);
>          create_from_location_async_data_free(data);
>
>          return;
> @@ -870,8 +863,8 @@ static void on_location_skipped(GObject *source,
>      g_input_stream_read_async(stream,
>                                (gchar *)&data->pvd,
>                                data->length,
> -                              data->priority,
> -                              data->cancellable,
> +                              g_task_get_priority(data->res),
> +                              g_task_get_cancellable(data->res),
>                                on_pvd_read,
>                                data);
>  }
> @@ -889,8 +882,7 @@ static void on_location_read(GObject *source,
>      stream = g_file_read_finish(G_FILE(source), res, &error);
>      if (error != NULL) {
>          g_prefix_error(&error, _("Failed to open file"));
> -        g_simple_async_result_take_error(data->res, error);
> -        g_simple_async_result_complete(data->res);
> +        g_task_return_error(data->res, error);
>          create_from_location_async_data_free(data);
>
>          return;
> @@ -898,8 +890,8 @@ static void on_location_read(GObject *source,
>
>      g_input_stream_skip_async(G_INPUT_STREAM(stream),
>                                PVD_OFFSET,
> -                              data->priority,
> -                              data->cancellable,
> +                              g_task_get_priority(data->res),
> +                              g_task_get_cancellable(data->res),
>                                on_location_skipped,
>                                data);
>  }
> @@ -925,14 +917,13 @@ void osinfo_media_create_from_location_async(const gchar *location,
>      g_return_if_fail(location != NULL);
>
>      data = g_slice_new0(CreateFromLocationAsyncData);
> -    data->res = g_simple_async_result_new
> -                                (NULL,
> -                                 callback,
> -                                 user_data,
> -                                 osinfo_media_create_from_location_async);
> +    data->res = g_task_new(NULL,
> +                           cancellable,
> +                           callback,
> +                           user_data);
> +    g_task_set_priority(data->res, priority);
> +
>      data->file = g_file_new_for_commandline_arg(location);
> -    data->priority = priority;
> -    data->cancellable = cancellable;
>      g_file_read_async(data->file,
>                        priority,
>                        cancellable,
> @@ -953,14 +944,11 @@ void osinfo_media_create_from_location_async(const gchar *location,
>  OsinfoMedia *osinfo_media_create_from_location_finish(GAsyncResult *res,
>                                                        GError **error)
>  {
> -    GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
> +    GTask *task = G_TASK(res);
>
>      g_return_val_if_fail(error == NULL || *error == NULL, NULL);
>
> -    if (g_simple_async_result_propagate_error(simple, error))
> -        return NULL;
> -
> -    return g_simple_async_result_get_op_res_gpointer(simple);
> +    return g_task_propagate_pointer(task, error);
>  }
>
>  /**
> diff --git a/osinfo/osinfo_tree.c b/osinfo/osinfo_tree.c
> index 55c572e..c80c62d 100644
> --- a/osinfo/osinfo_tree.c
> +++ b/osinfo/osinfo_tree.c
> @@ -36,10 +36,7 @@ struct _CreateFromLocationAsyncData {
>      GFile *file;
>      gchar *location;
>
> -    gint priority;
> -    GCancellable *cancellable;
> -
> -    GSimpleAsyncResult *res;
> +    GTask *res;
>
>      OsinfoTree *tree;
>  };
> @@ -49,7 +46,6 @@ static void create_from_location_async_data_free(CreateFromLocationAsyncData *da
>      if (data->tree)
>      g_object_unref(data->tree);
>      g_object_unref(data->file);
> -    g_clear_object(&data->cancellable);
>      g_object_unref(data->res);
>
>      g_slice_free(CreateFromLocationAsyncData, data);
> @@ -604,8 +600,7 @@ static void on_location_read(GObject *source,
>                                       NULL,
>                                       &error)) {
>          g_prefix_error(&error, _("Failed to load .treeinfo file: "));
> -        g_simple_async_result_take_error(data->res, error);
> -        g_simple_async_result_complete(data->res);
> +        g_task_return_error(data->res, error);
>          create_from_location_async_data_free(data);
>          return;
>      }
> @@ -615,14 +610,13 @@ static void on_location_read(GObject *source,
>                               length,
>                               &error))) {
>          g_prefix_error(&error, _("Failed to process keyinfo file: "));
> -        g_simple_async_result_take_error(data->res, error);
> +        g_task_return_error(data->res, error);
>          goto cleanup;
>      }
>
> -    g_simple_async_result_set_op_res_gpointer(data->res, ret, NULL);
> +    g_task_return_pointer(data->res, ret, NULL);
>
>   cleanup:
> -    g_simple_async_result_complete(data->res);
>      create_from_location_async_data_free(data);
>      g_free(content);
>  }
> @@ -651,15 +645,14 @@ void osinfo_tree_create_from_location_async(const gchar *location,
>      treeinfo = g_strdup_printf("%s/.treeinfo", location);
>
>      data = g_slice_new0(CreateFromLocationAsyncData);
> -    data->res = g_simple_async_result_new
> -        (NULL,
> -         callback,
> -         user_data,
> -         osinfo_tree_create_from_location_async);
> +    data->res = g_task_new(NULL,
> +                           cancellable,
> +                           callback,
> +                           user_data);
> +    g_task_set_priority(data->res, priority);
> +
>      data->file = g_file_new_for_uri(treeinfo);
>      data->location = g_strdup(location);
> -    data->priority = priority;
> -    data->cancellable = cancellable;
>
>      /* XXX priority ? */
>      /* XXX probe other things besides just tree info */
> @@ -685,14 +678,11 @@ void osinfo_tree_create_from_location_async(const gchar *location,
>  OsinfoTree *osinfo_tree_create_from_location_finish(GAsyncResult *res,
>                                                      GError **error)
>  {
> -    GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(res);
> +    GTask *task = G_TASK(res);
>
>      g_return_val_if_fail(error == NULL || *error == NULL, NULL);
>
> -    if (g_simple_async_result_propagate_error(simple, error))
> -        return NULL;
> -
> -    return g_simple_async_result_get_op_res_gpointer(simple);
> +    return g_task_propagate_pointer(task, error);
>  }
>
>  /**
> --
> 2.5.0
>
> _______________________________________________
> Libosinfo mailing list
> Libosinfo@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libosinfo



-- 
Regards,

Zeeshan Ali (Khattak)
________________________________________
Befriend GNOME: http://www.gnome.org/friends/

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux