GNOME Bugzilla – Bug 760581
Port pp_cups_get_dests* to GTask
Last modified: 2016-01-15 14:22:31 UTC
.
Created attachment 318973 [details] [review] printers: port pp_cups_get_dests* to GTask As of GLib 2.46, GSimpleAsyncResult is deprecated in favor of GTask, which provides a simpler API.
Review of attachment 318973 [details] [review]: Thank you for converting this to GTask. I have few comments. ::: panels/printers/pp-cups.c @@ +81,3 @@ + + if (data->dests->num_of_dests) + g_task_return_pointer (task, data->dests, (GDestroyNotify) cgd_data_free); The cgd_data_free() is for freeing of CGDData structure not for freeing of PpCupsDests. @@ +83,3 @@ + g_task_return_pointer (task, data->dests, (GDestroyNotify) cgd_data_free); + else + g_task_return_error (task, error); Having 0 dests is not an error (according to me not according to CUPS). If I remember correctly this was the reason to have the PpCupsDests inside of the CGDData instead of just PpCupsDests so we know that the NULL was set by the cupsGetDests(). Looking into CUPS source code, they consider 0 dests as an error so we can not detect whether there was an error or not. So lets just return the PpCupsDests and remove the whole CGDData structure (not using task data then) and don't set error when there is 0 dests, just return NULL.
Created attachment 319009 [details] [review] printers: port pp_cups_get_dests* to GTask As of GLib 2.46, GSimpleAsyncResult is deprecated in favor of GTask, which provides a simpler API.
Review of attachment 319009 [details] [review]: ::: panels/printers/pp-cups.c @@ +61,2 @@ { + PpCupsDests *dests = task_data; You set the pointer on next line so the initialization is not used. @@ +82,2 @@ + task = g_task_new (cups, cancellable, callback, user_data); + g_task_set_task_data (task, dests, (GDestroyNotify) pp_cups_dests_free); You don't need the task data at all.
Created attachment 319085 [details] [review] printers: port pp_cups_get_dests* to GTask As of GLib 2.46, GSimpleAsyncResult is deprecated in favor of GTask, which provides a simpler API.
Review of attachment 319085 [details] [review]: ::: panels/printers/pp-cups.c @@ +79,2 @@ + dests = g_new0 (PpCupsDests, 1); + dests->dests = NULL; I'm sorry, I did not mention it specifically in the previous review, but you also don't need the dests pointer here because you set it in the _pp_cups_get_dests_thread(). So remove "PpCupsDests *dests;" and its initialization.
Created attachment 319086 [details] [review] printers: port pp_cups_get_dests* to GTask As of GLib 2.46, GSimpleAsyncResult is deprecated in favor of GTask, which provides a simpler API.
Review of attachment 319086 [details] [review]: Thank you for the changes. The patch looks good to me now.
Comment on attachment 319086 [details] [review] printers: port pp_cups_get_dests* to GTask Thanks. Pushed to master as https://git.gnome.org/browse/gnome-control-center/commit/?id=7cd6800da0b343aaa44bdafb7e1f2348995aa414