[Spice-devel] [PATCH v2 11/13] vmcstream: Use GTask instead of GSimpleAsyncResult
Jonathon Jongsma
jjongsma at redhat.com
Tue Feb 16 23:09:05 UTC 2016
Mostly OK, a couple minor comments / questions
On Fri, 2016-02-12 at 10:46 +0100, Fabiano FidĂȘncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
> src/vmcstream.c | 129 +++++++++++++++++++++++--------------------------------
> -
> 1 file changed, 52 insertions(+), 77 deletions(-)
>
> diff --git a/src/vmcstream.c b/src/vmcstream.c
> index 483dd5a..05dba7d 100644
> --- a/src/vmcstream.c
> +++ b/src/vmcstream.c
> @@ -27,7 +27,7 @@
> struct _SpiceVmcInputStream
> {
> GInputStream parent_instance;
> - GSimpleAsyncResult *result;
> + GTask *task;
> struct coroutine *coroutine;
>
> SpiceChannel *channel;
> @@ -36,7 +36,6 @@ struct _SpiceVmcInputStream
> gsize count;
> gsize pos;
>
> - GCancellable *cancellable;
> gulong cancel_id;
> };
>
> @@ -118,11 +117,12 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
> *self,
> self->coroutine = coroutine_self();
>
> while (size > 0) {
> - SPICE_DEBUG("spicevmc co_data %p", self->result);
> - if (!self->result)
> + GCancellable *cancellable;
> + SPICE_DEBUG("spicevmc co_data %p", self->task);
> + if (!self->task)
> coroutine_yield(NULL);
>
> - g_return_if_fail(self->result != NULL);
> + g_return_if_fail(self->task != NULL);
>
> gsize min = MIN(self->count, size);
> memcpy(self->buffer, data, min);
> @@ -139,14 +139,13 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
> *self,
> if (self->all && min > 0 && self->pos != self->count)
> continue;
>
> - g_simple_async_result_set_op_res_gssize(self->result, self->pos);
> + g_task_return_int(self->task, self->pos);
> +
> + cancellable = g_task_get_cancellable(self->task);
> + if (cancellable)
> + g_cancellable_disconnect(cancellable, self->cancel_id);
>
> - g_simple_async_result_complete_in_idle(self->result);
> - g_clear_object(&self->result);
> - if (self->cancellable) {
> - g_cancellable_disconnect(self->cancellable, self->cancel_id);
> - g_clear_object(&self->cancellable);
> - }
> + g_clear_object(&self->task);
> }
>
> self->coroutine = NULL;
> @@ -158,13 +157,12 @@ read_cancelled(GCancellable *cancellable,
> {
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(user_data);
>
> - SPICE_DEBUG("read cancelled, %p", self->result);
> - g_simple_async_result_set_error(self->result,
> - G_IO_ERROR, G_IO_ERROR_CANCELLED,
> - "read cancelled");
> - g_simple_async_result_complete_in_idle(self->result);
> + SPICE_DEBUG("read cancelled, %p", self->task);
> + g_task_return_new_error(self->task,
> + G_IO_ERROR, G_IO_ERROR_CANCELLED,
> + "read cancelled");
Hmm, doesn't GTask handle this for us?
>
> - g_clear_object(&self->result);
> + g_clear_object(&self->task);
>
> /* See FIXME */
> /* if (self->cancellable) { */
> @@ -183,21 +181,20 @@ spice_vmc_input_stream_read_all_async(GInputStream
> *stream,
> gpointer user_data)
> {
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
> - GSimpleAsyncResult *result;
> + GTask *task;
>
> /* no concurrent read permitted by ginputstream */
> - g_return_if_fail(self->result == NULL);
> - g_return_if_fail(self->cancellable == NULL);
> + g_return_if_fail(self->task == NULL);
> + g_return_if_fail(g_task_get_cancellable(self->task) == NULL);
This second line should be removed. The line above ensures that self->task is
NULL, so you shouldn't call a member function on a NULL pointer.
> self->all = TRUE;
> self->buffer = buffer;
> self->count = count;
> self->pos = 0;
> - result = g_simple_async_result_new(G_OBJECT(self),
> - callback,
> - user_data,
> - spice_vmc_input_stream_read_async);
> - self->result = result;
> - self->cancellable = g_object_ref(cancellable);
> + task = g_task_new(self,
> + cancellable,
> + callback,
> + user_data);
> + self->task = task;
> if (cancellable)
> self->cancel_id =
> g_cancellable_connect(cancellable, G_CALLBACK(read_cancelled),
> self, NULL);
> @@ -211,27 +208,19 @@ spice_vmc_input_stream_read_all_finish(GInputStream
> *stream,
> GAsyncResult *result,
> GError **error)
> {
> - GSimpleAsyncResult *simple;
> + GTask *task = G_TASK(result);
> + GCancellable *cancellable;
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
>
> - g_return_val_if_fail(g_simple_async_result_is_valid(result,
> - G_OBJECT(self),
> -
> spice_vmc_input_stream_read_async),
> - -1);
> -
> - simple = (GSimpleAsyncResult *)result;
> + g_return_val_if_fail(g_task_is_valid(task, self), -1);
>
> /* FIXME: calling _finish() is required. Disconnecting in
> read_cancelled() causes a deadlock. #705395 */
> - if (self->cancellable) {
> - g_cancellable_disconnect(self->cancellable, self->cancel_id);
> - g_clear_object(&self->cancellable);
> - }
> -
> - if (g_simple_async_result_propagate_error(simple, error))
> - return -1;
> + cancellable = g_task_get_cancellable(task);
> + if (cancellable)
> + g_cancellable_disconnect(cancellable, self->cancel_id);
>
> - return g_simple_async_result_get_op_res_gssize(simple);
> + return g_task_propagate_int(task, error);
> }
>
> static void
> @@ -244,21 +233,18 @@ spice_vmc_input_stream_read_async(GInputStream
> *stream,
> gpointer user_data)
> {
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
> - GSimpleAsyncResult *result;
> + GTask *task;
>
> /* no concurrent read permitted by ginputstream */
> - g_return_if_fail(self->result == NULL);
> - g_return_if_fail(self->cancellable == NULL);
> + g_return_if_fail(self->task == NULL);
> + g_return_if_fail(g_task_get_cancellable(self->task) == NULL);
Same as above, remove second line.
> self->all = FALSE;
> self->buffer = buffer;
> self->count = count;
> self->pos = 0;
> - result = g_simple_async_result_new(G_OBJECT(self),
> - callback,
> - user_data,
> - spice_vmc_input_stream_read_async);
> - self->result = result;
> - self->cancellable = g_object_ref(cancellable);
> +
> + task = g_task_new(self, cancellable, callback, user_data);
> + self->task = task;
> if (cancellable)
> self->cancel_id =
> g_cancellable_connect(cancellable, G_CALLBACK(read_cancelled),
> self, NULL);
> @@ -272,27 +258,19 @@ spice_vmc_input_stream_read_finish(GInputStream *stream,
> GAsyncResult *result,
> GError **error)
> {
> - GSimpleAsyncResult *simple;
> + GTask *task = G_TASK(result);
> + GCancellable *cancellable;
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream);
>
> - g_return_val_if_fail(g_simple_async_result_is_valid(result,
> - G_OBJECT(self),
> -
> spice_vmc_input_stream_read_async),
> - -1);
> -
> - simple = (GSimpleAsyncResult *)result;
> + g_return_val_if_fail(g_task_is_valid(task, self), -1);
>
> /* FIXME: calling _finish() is required. Disconnecting in
> read_cancelled() causes a deadlock. #705395 */
> - if (self->cancellable) {
> - g_cancellable_disconnect(self->cancellable, self->cancel_id);
> - g_clear_object(&self->cancellable);
> - }
> -
> - if (g_simple_async_result_propagate_error(simple, error))
> - return -1;
> + cancellable = g_task_get_cancellable(task);
> + if (cancellable)
> + g_cancellable_disconnect(cancellable, self->cancel_id);
>
> - return g_simple_async_result_get_op_res_gssize(simple);
> + return g_task_propagate_int(task, error);
> }
>
> static gssize
> @@ -407,11 +385,10 @@ spice_vmc_output_stream_write_finish(GOutputStream
> *stream,
> GError **error)
> {
> SpiceVmcOutputStream *self = SPICE_VMC_OUTPUT_STREAM(stream);
> - GSimpleAsyncResult *res =
> -
> g_simple_async_result_get_op_res_gpointer(G_SIMPLE_ASYNC_RESULT(simple));
> + GAsyncResult *res = g_task_propagate_pointer(G_TASK(simple), error);
>
> SPICE_DEBUG("spicevmc write finish");
> - return spice_vmc_write_finish(self->channel, G_ASYNC_RESULT(res), error);
> + return spice_vmc_write_finish(self->channel, res, error);
> }
>
> static void
> @@ -419,12 +396,11 @@ write_cb(GObject *source_object,
> GAsyncResult *res,
> gpointer user_data)
> {
> - GSimpleAsyncResult *simple = user_data;
> + GTask *task = user_data;
>
> - g_simple_async_result_set_op_res_gpointer(simple, res, NULL);
> + g_task_return_pointer(task, g_object_ref(task), g_object_unref);
>
> - g_simple_async_result_complete(simple);
> - g_object_unref(simple);
> + g_object_unref(task);
> }
>
> static void
> @@ -437,16 +413,15 @@ spice_vmc_output_stream_write_async(GOutputStream
> *stream,
> gpointer user_data)
> {
> SpiceVmcOutputStream *self = SPICE_VMC_OUTPUT_STREAM(stream);
> - GSimpleAsyncResult *simple;
> + GTask *task;
>
> SPICE_DEBUG("spicevmc write async");
> /* an AsyncResult to forward async op to channel */
> - simple = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
> - spice_vmc_output_stream_write_async);
> + task = g_task_new(self, cancellable, callback, user_data);
>
> spice_vmc_write_async(self->channel, buffer, count,
> cancellable, write_cb,
> - simple);
> + task);
> }
>
> /* STREAM */
Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
More information about the Spice-devel
mailing list