Re: [libvirt-glib v5] gobject: Port to GTask API

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

 



Hey,

Still a couple of comments :(

On Wed, Nov 25, 2015 at 09:46:58PM +0000, Zeeshan Ali (Khattak) wrote:
> diff --git a/libvirt-gobject/libvirt-gobject-input-stream.c b/libvirt-gobject/libvirt-gobject-input-stream.c
> index ff1a70c..8ef60d6 100644
> --- a/libvirt-gobject/libvirt-gobject-input-stream.c
> +++ b/libvirt-gobject/libvirt-gobject-input-stream.c
> @@ -103,48 +102,45 @@ gvir_input_stream_read_ready(GVirStream *stream,
>  {
>      GVirInputStream *input_stream = GVIR_INPUT_STREAM(opaque);
>      GVirInputStreamPrivate *priv = input_stream->priv;
> -    GSimpleAsyncResult *simple = priv->result;
> +    GTask *task = priv->task;
> +    GCancellable *cancellable = g_task_get_cancellable(task);
>      GError *error = NULL;
>      gssize result;
>  
> +    priv->task = NULL;
> +
>      if (!(cond & GVIR_STREAM_IO_CONDITION_READABLE)) {
>          g_warn_if_reached();
> -        g_simple_async_result_set_error(simple,
> -                                        G_IO_ERROR,
> -                                        G_IO_ERROR_INVALID_ARGUMENT,
> -                                        "%s",
> -                                        "Expected stream to be readable");
> +        g_task_return_new_error(task,
> +                                G_IO_ERROR,
> +                                G_IO_ERROR_INVALID_ARGUMENT,
> +                                "%s",
> +                                "Expected stream to be readable");
>          goto cleanup;
>      }
>  
> -    result  = gvir_stream_receive(stream, priv->buffer, priv->count,
> -                                  priv->cancellable, &error);
> +    result = gvir_stream_receive(stream, priv->buffer, priv->count,
> +                                 cancellable, &error);
> +    if (error != NULL) {
> +        if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> +            g_warn_if_reached();
> +            g_task_return_new_error(task,
> +                                    G_IO_ERROR,
> +                                    G_IO_ERROR_INVALID_ARGUMENT,
> +                                    "%s",
> +                                    "Expected stream to be readable");
> +        } else {
> +            g_task_return_error(task, error);
> +        }
> +        g_error_free (error);

g_task_return_error() takes ownership of its GError argument, so the
code should be:
    if (error != NULL) {
        if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
            g_warn_if_reached();
            g_task_return_new_error(task,
                                    G_IO_ERROR,
                                    G_IO_ERROR_INVALID_ARGUMENT,
                                    "%s",
                                    "Expected stream to be readable");
            g_error_free (error);
        } else {
            g_task_return_error(task, error);
        }
    }

> diff --git a/libvirt-gobject/libvirt-gobject-output-stream.c b/libvirt-gobject/libvirt-gobject-output-stream.c
> index f39328b..6968c3d 100644
> --- a/libvirt-gobject/libvirt-gobject-output-stream.c
> +++ b/libvirt-gobject/libvirt-gobject-output-stream.c
> @@ -103,48 +102,45 @@ gvir_output_stream_write_ready(GVirStream *stream,
>  {
>      GVirOutputStream *output_stream = GVIR_OUTPUT_STREAM(opaque);
>      GVirOutputStreamPrivate *priv = output_stream->priv;
> -    GSimpleAsyncResult *simple = priv->result;
> +    GTask *task = priv->task;
> +    GCancellable *cancellable = g_task_get_cancellable(task);
>      GError *error = NULL;
>      gssize result;
>  
>      if (!(cond & GVIR_STREAM_IO_CONDITION_WRITABLE)) {
>          g_warn_if_reached();
> -        g_simple_async_result_set_error(simple,
> -                                        G_IO_ERROR,
> -                                        G_IO_ERROR_INVALID_ARGUMENT,
> -                                        "%s",
> -                                        "Expected stream to be writable");
> +        g_task_return_new_error(task,
> +                                G_IO_ERROR,
> +                                G_IO_ERROR_INVALID_ARGUMENT,
> +                                "%s",
> +                                "Expected stream to be readable");
>          goto cleanup;
>      }
>  
>      result  = gvir_stream_send(stream, priv->buffer, priv->count,
> -                               priv->cancellable, &error);
> +                               cancellable, &error);
> +
> +    if (error != NULL) {
> +        if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> +            g_warn_if_reached();
> +            g_task_return_new_error(task,
> +                                    G_IO_ERROR,
> +                                    G_IO_ERROR_INVALID_ARGUMENT,
> +                                    "%s",
> +                                    "Expected stream to be writable");
> +        } else {
> +            g_task_return_error(task, error);
> +        }
> +        g_error_free (error);

Same comment here

> diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c
> index 46dbd9a..2bb3e20 100644
> --- a/libvirt-gobject/libvirt-gobject-stream.c
> +++ b/libvirt-gobject/libvirt-gobject-stream.c
> @@ -146,27 +143,42 @@ static void gvir_stream_close_async(GIOStream *stream,
>      error = NULL;
>      if (class->close_fn &&
>          !class->close_fn(stream, cancellable, &error)) {
> -        g_simple_async_report_take_gerror_in_idle(G_OBJECT (stream),
> -                                                  callback, user_data,
> -                                                  error);
> -        return;
> +        g_task_return_error(task, error);
> +
> +        goto exit;
>      }
>  
> -    res = g_simple_async_result_new(G_OBJECT (stream),
> -                                    callback,
> -                                    user_data,
> -                                    gvir_stream_close_async);
> -    g_simple_async_result_complete_in_idle(res);
> -    g_object_unref (res);
> +    g_task_return_boolean(task, TRUE);
> +exit:
> +    g_object_unref(task);
> +    return FALSE;
>  }
>  
> +static void gvir_stream_close_async(GIOStream *stream,
> +                                    int io_priority G_GNUC_UNUSED,
> +                                    GCancellable *cancellable,
> +                                    GAsyncReadyCallback callback,
> +                                    gpointer user_data)
> +{
> +    GTask *task;
> +
> +    task = g_task_new(G_OBJECT(stream),
> +                      cancellable,
> +                      callback,
> +                      user_data);
> +    g_idle_add(close_in_idle, task);
> +}
>  
>  static gboolean
> -gvir_stream_close_finish(GIOStream *stream G_GNUC_UNUSED,
> -                         GAsyncResult *result G_GNUC_UNUSED,
> -                         GError **error G_GNUC_UNUSED)
> +gvir_stream_close_finish(GIOStream *stream,
> +                         GAsyncResult *result,
> +                         GError **error)
>  {
> -    return TRUE;
> +    g_return_val_if_fail(GVIR_IS_STREAM(stream), -1);
> +    g_return_val_if_fail(g_task_is_valid(result, stream), -1);
> +    g_return_val_if_fail(error == NULL || *error == NULL, -1);

g_return_val_if_fail(.., FALSE);

ACK with these issues fixed.

Christophe

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

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