Re: [Spice-devel] [PATCH phodav 11/13] spice: move OutputQueue to file

2019-05-23 Thread Marc-André Lureau 
Hi

On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> OutputQueue is a self-contained unit and as such can be put in
> a separate file to make the spice-webdavd.c less cluttered.
>
> Also, as the current implementation defines output_queue_{ref, unref},
> turn OutputQueue into a GObject which can handle these for us.
>
> Signed-off-by: Jakub Janků 

ack in principle, minor coding style issues

The phodav source code tries to follow glib code style. Can you indent
accordingly?

> ---
>  spice/meson.build |   8 ++-
>  spice/output-queue.c  | 164 ++
>  spice/output-queue.h  |  38 ++
>  spice/spice-webdavd.c | 162 ++---
>  4 files changed, 214 insertions(+), 158 deletions(-)
>  create mode 100644 spice/output-queue.c
>  create mode 100644 spice/output-queue.h
>
> diff --git a/spice/meson.build b/spice/meson.build
> index 6db22cc..06d20e6 100644
> --- a/spice/meson.build
> +++ b/spice/meson.build
> @@ -4,9 +4,15 @@ if host_machine.system() == 'windows'
>win32_deps += compiler.find_library('mpr')
>  endif
>
> +sources = [
> +  'spice-webdavd.c',
> +  'output-queue.c',
> +  'output-queue.h'
> +]
> +
>  executable(
>'spice-webdavd',
> -  [ 'spice-webdavd.c' ],
> +  sources,
>install_dir : sbindir,
>include_directories : incdir,
>dependencies : win32_deps + avahi_deps + deps,
> diff --git a/spice/output-queue.c b/spice/output-queue.c
> new file mode 100644
> index 000..6991493
> --- /dev/null
> +++ b/spice/output-queue.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2019 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +#include 
> +
> +#include "output-queue.h"
> +
> +typedef struct _OutputQueueElem
> +{
> +  OutputQueue  *queue;
> +  const guint8 *buf;
> +  gsize size;
> +  PushedCb  cb;
> +  gpointer  user_data;
> +} OutputQueueElem;
> +
> +struct _OutputQueue
> +{
> +  GObjectparent_instance;
> +  GOutputStream *output;
> +  gboolean   flushing;
> +  guint  idle_id;
> +  GQueue*queue;
> +  GCancellable  *cancel;
> +};
> +
> +G_DEFINE_TYPE(OutputQueue, output_queue, G_TYPE_OBJECT);
> +
> +static void output_queue_init(OutputQueue *self)
> +{
> +self->queue = g_queue_new ();
> +}
> +
> +static void output_queue_finalize(GObject *obj)
> +{
> +OutputQueue *self = OUTPUT_QUEUE(obj);
> +
> +g_warn_if_fail (g_queue_get_length (self->queue) == 0);
> +g_warn_if_fail (!self->flushing);
> +g_warn_if_fail (!self->idle_id);
> +
> +g_queue_free_full (self->queue, g_free);
> +g_object_unref (self->output);
> +g_object_unref (self->cancel);
> +
> +G_OBJECT_CLASS(output_queue_parent_class)->finalize(obj);
> +}
> +
> +static void output_queue_class_init(OutputQueueClass *klass)
> +{
> +GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
> +gobject_class->finalize = output_queue_finalize;
> +}
> +
> +OutputQueue* output_queue_new (GOutputStream *output, GCancellable *cancel)
> +{
> +  OutputQueue *self = g_object_new(OUTPUT_TYPE_QUEUE, NULL);
> +  self->output = g_object_ref (output);
> +  self->cancel = g_object_ref (cancel);
> +  return self;
> +}
> +
> +static gboolean output_queue_idle (gpointer user_data);
> +
> +static void
> +output_queue_flush_cb (GObject  *source_object,
> +   GAsyncResult *res,
> +   gpointer  user_data)
> +{
> +  GError *error = NULL;
> +  OutputQueueElem *e = user_data;
> +  OutputQueue *q = e->queue;
> +
> +  g_debug ("flushed");
> +  q->flushing = FALSE;
> +  g_output_stream_flush_finish (G_OUTPUT_STREAM (source_object),
> +res, );
> +  if (error)
> +g_warning ("error: %s", error->message);
> +
> +  g_clear_error ();
> +
> +  if (!q->idle_id)
> +q->idle_id = g_idle_add (output_queue_idle, g_object_ref (q));
> +
> +  g_free (e);
> +  g_object_unref (q);
> +}
> +
> +static gboolean
> +output_queue_idle (gpointer user_data)
> +{
> +  OutputQueue *q = user_data;
> +  OutputQueueElem *e = NULL;
> +  GError *error = NULL;
> +
> +  if (q->flushing)
> +{
> +  g_debug ("already flushing");
> +  goto end;
> +}
> +
> +  e = g_queue_pop_head (q->queue);
> +  if (!e)
> +{
> +  g_debug ("No more data to flush");
> +

[Spice-devel] [PATCH phodav 11/13] spice: move OutputQueue to file

2019-05-23 Thread Jakub Janků 
OutputQueue is a self-contained unit and as such can be put in
a separate file to make the spice-webdavd.c less cluttered.

Also, as the current implementation defines output_queue_{ref, unref},
turn OutputQueue into a GObject which can handle these for us.

Signed-off-by: Jakub Janků 
---
 spice/meson.build |   8 ++-
 spice/output-queue.c  | 164 ++
 spice/output-queue.h  |  38 ++
 spice/spice-webdavd.c | 162 ++---
 4 files changed, 214 insertions(+), 158 deletions(-)
 create mode 100644 spice/output-queue.c
 create mode 100644 spice/output-queue.h

diff --git a/spice/meson.build b/spice/meson.build
index 6db22cc..06d20e6 100644
--- a/spice/meson.build
+++ b/spice/meson.build
@@ -4,9 +4,15 @@ if host_machine.system() == 'windows'
   win32_deps += compiler.find_library('mpr')
 endif
 
+sources = [
+  'spice-webdavd.c',
+  'output-queue.c',
+  'output-queue.h'
+]
+
 executable(
   'spice-webdavd',
-  [ 'spice-webdavd.c' ],
+  sources,
   install_dir : sbindir,
   include_directories : incdir,
   dependencies : win32_deps + avahi_deps + deps,
diff --git a/spice/output-queue.c b/spice/output-queue.c
new file mode 100644
index 000..6991493
--- /dev/null
+++ b/spice/output-queue.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+#include 
+
+#include "output-queue.h"
+
+typedef struct _OutputQueueElem
+{
+  OutputQueue  *queue;
+  const guint8 *buf;
+  gsize size;
+  PushedCb  cb;
+  gpointer  user_data;
+} OutputQueueElem;
+
+struct _OutputQueue
+{
+  GObjectparent_instance;
+  GOutputStream *output;
+  gboolean   flushing;
+  guint  idle_id;
+  GQueue*queue;
+  GCancellable  *cancel;
+};
+
+G_DEFINE_TYPE(OutputQueue, output_queue, G_TYPE_OBJECT);
+
+static void output_queue_init(OutputQueue *self)
+{
+self->queue = g_queue_new ();
+}
+
+static void output_queue_finalize(GObject *obj)
+{
+OutputQueue *self = OUTPUT_QUEUE(obj);
+
+g_warn_if_fail (g_queue_get_length (self->queue) == 0);
+g_warn_if_fail (!self->flushing);
+g_warn_if_fail (!self->idle_id);
+
+g_queue_free_full (self->queue, g_free);
+g_object_unref (self->output);
+g_object_unref (self->cancel);
+
+G_OBJECT_CLASS(output_queue_parent_class)->finalize(obj);
+}
+
+static void output_queue_class_init(OutputQueueClass *klass)
+{
+GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
+gobject_class->finalize = output_queue_finalize;
+}
+
+OutputQueue* output_queue_new (GOutputStream *output, GCancellable *cancel)
+{
+  OutputQueue *self = g_object_new(OUTPUT_TYPE_QUEUE, NULL);
+  self->output = g_object_ref (output);
+  self->cancel = g_object_ref (cancel);
+  return self;
+}
+
+static gboolean output_queue_idle (gpointer user_data);
+
+static void
+output_queue_flush_cb (GObject  *source_object,
+   GAsyncResult *res,
+   gpointer  user_data)
+{
+  GError *error = NULL;
+  OutputQueueElem *e = user_data;
+  OutputQueue *q = e->queue;
+
+  g_debug ("flushed");
+  q->flushing = FALSE;
+  g_output_stream_flush_finish (G_OUTPUT_STREAM (source_object),
+res, );
+  if (error)
+g_warning ("error: %s", error->message);
+
+  g_clear_error ();
+
+  if (!q->idle_id)
+q->idle_id = g_idle_add (output_queue_idle, g_object_ref (q));
+
+  g_free (e);
+  g_object_unref (q);
+}
+
+static gboolean
+output_queue_idle (gpointer user_data)
+{
+  OutputQueue *q = user_data;
+  OutputQueueElem *e = NULL;
+  GError *error = NULL;
+
+  if (q->flushing)
+{
+  g_debug ("already flushing");
+  goto end;
+}
+
+  e = g_queue_pop_head (q->queue);
+  if (!e)
+{
+  g_debug ("No more data to flush");
+  goto end;
+}
+
+  g_debug ("flushing %" G_GSIZE_FORMAT, e->size);
+  g_output_stream_write_all (q->output, e->buf, e->size, NULL, q->cancel, 
);
+  if (e->cb)
+e->cb (q, e->user_data, error);
+
+  if (error)
+  goto end;
+
+  q->flushing = TRUE;
+  g_output_stream_flush_async (q->output, G_PRIORITY_DEFAULT, q->cancel, 
output_queue_flush_cb, e);
+
+  q->idle_id = 0;
+  return FALSE;
+
+end:
+  g_clear_error ();
+  q->idle_id = 0;
+  g_free (e);
+  g_object_unref (q);
+
+  return FALSE;
+}
+
+void