date:20160118

Maybe it's just me, but I don't really understand the purpose of this patch. Can
you give a big-picture explanation?




On Thu, 2016-01-14 at 22:01 +0100, Marc-André Lureau wrote:
> From: Marc-André Lureau 
> 
> Send the fd associated to the last message sent.
> 
> Even if the fd is invalid, the sendfd msg is appended to the protocol,
> for 2 reasons:
> - trying to send an invalid fd does not have to close the connection (it
>   would with an invalid fd)
> - even if the fd is invalid, the protocol expects an extra byte for the
>   ancillary data
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  server/red-channel.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 306c87d..b33c91d 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -608,8 +608,24 @@ static inline void
> red_channel_client_release_sent_item(RedChannelClient *rcc)
>  static void red_channel_peer_on_out_msg_done(void *opaque)
>  {
>  RedChannelClient *rcc = (RedChannelClient *)opaque;
> +int fd = spice_marshaller_get_fd(rcc->send_data.marshaller);
>  
>  rcc->send_data.size = 0;
> +
> +if (fd != -1) {
> +if (fcntl(fd, F_GETFD) == -1) {
> +close(fd);
> +fd = -1;
> +}
> +
> +if (reds_stream_send_msgfd(rcc->stream, fd) < 0) {
> +perror("sendfd");
> +red_channel_client_disconnect(rcc);
> +return;
> +}
> +close(fd);
> +}
> +
>  red_channel_client_release_sent_item(rcc);
>  if (rcc->send_data.blocked) {
>  rcc->send_data.blocked = FALSE;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH server v2 03/13] tests: make sure all tests are built on default rule

On Fri, 2016-01-15 at 09:08 -0500, Frediano Ziglio wrote:
> > 
> > Hi
> > 
> > - Original Message -
> > > > 
> > > > Hi
> > > > 
> > > > On Fri, Jan 15, 2016 at 11:50 AM, Frediano Ziglio 
> > > > wrote:
> > > > > 
> > > > > > Subject: [PATCH server v2 03/13] tests: make sure all tests are
> > > > > > built
> > > > > > on
> > > > > > default rule
> > > > > > 
> > > > > > We should have all or none, I don't see the point of having just
> > > > > > some
> > > > > > of
> > > > > > them.
> > > > > > 
> > > > > 
> > > > > I don't know why but this description (and subject) looks odd to me.
> > > > > 
> > > > > I would put something like (if I understood it):
> > > > > 
> > > > > "Programs listed in noinst_PROGRAMS inherit default rule/settings to
> > > > > list
> > > > > all
> > > > > tests program also in this macro"
> > > > > 
> > > > 
> > > > I don't mind changing the text, but I really don't understand yours ;)
> > > > 
> > > 
> > > That means I didn't get the reason of the change.
> > > 
> > > What's a rule for you?
> > > 
> > > output: source
> > >   do_something_to_generate_output_from_source
> > > 
> > > is a rule
> > > 
> > > TESTS = xxx
> > > 
> > > is a variable (also called macro)
> > 
> > Yes
> > 
> > the default rule being "all", it builds noinst_PROGRAMS but not
> > check_PROGRAMS. I prefer either build all with noinst_PROGRAMS or none (in
> > which case we should remove some of them duplicated in check_PROGRAMS)
> > 
> 
> Really didn't get it before :)
> Can you post again with an updated comment?
> 
> Frediano


I didn't fully understand the original comment either (nor did I really
understand Frediano's suggested alternative ;). 

I think that even a simple re-wording would make it a little more clear:

"Make sure that the default rule builds all tests."


> 
> > > 
> > > > > > Signed-off-by: Marc-André Lureau 
> > > > > > ---
> > > > > >  server/tests/Makefile.am | 15 +++
> > > > > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> > > > > > index 15196f9..007930c 100644
> > > > > > --- a/server/tests/Makefile.am
> > > > > > +++ b/server/tests/Makefile.am
> > > > > > @@ -35,6 +35,12 @@ LDADD =
> > > > > > \
> > > > > >   $(SPICE_NONPKGCONFIG_LIBS)  \
> > > > > >   $(NULL)
> > > > > > 
> > > > > > +TESTS =  \
> > > > > > + stat_test   \
> > > > > > + stream-test \
> > > > > > + test-qxl-parsing\
> > > > > > + $(NULL)
> > > > > > +
> > > > > 
> > > > > Why did you moved above? This just make the patch bigger.
> > > > > 
> > > > 
> > > > It's saner/simpler to define before use.
> > > > 
> > > 
> > > Let's say that is more similar to assignment and other languages so
> > > it's easier to read by people not very friendly with Makefiles
> > > (and honestly I think that many developers does not really like
> > > Makefiles).
> > > 
> > > > > >  noinst_PROGRAMS =\
> > > > > >   test_display_no_ssl \
> > > > > >   test_display_streaming  \
> > > > > > @@ -47,14 +53,7 @@ noinst_PROGRAMS =  \
> > > > > >   test_vdagent\
> > > > > >   test_display_width_stride   \
> > > > > >   spice-server-replay \
> > > > > > - stream-test \
> > > > > > - stat_test   \
> > > > > > - $(NULL)
> > > > > > -
> > > > > > -TESTS =  \
> > > > > > - stat_test   \
> > > > > > - test-qxl-parsing\
> > > > > > - stream-test \
> > > > > > + $(TESTS)\
> > > > > >   $(NULL)
> > > > > > 
> > > > > >  check_PROGRAMS = $(TESTS)
> > > > > > --
> > > > > > 2.5.0
> > > > > > 
> > > > > > 
> > > > > 
> > > > > Frediano
> > > > 
> > > > 
> > > > 
> > > > --
> > > > Marc-André Lureau
> > > > 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH server v2 02/13] tests: link test-qxl-parsing with libserver

In general, I appreciate a bit more justification in the commit log so I can
quickly understand the "why" in addition to the "what". In this case, I assume
it's to reduce build time and avoid compiling these files twice. ACK with a
slightly expanded commit log.

Acked-by: Jonathon Jongsma 



On Thu, 2016-01-14 at 22:01 +0100, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  server/tests/Makefile.am | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 8caff04..15196f9 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -91,8 +91,4 @@ libstat_test3_a_CPPFLAGS = $(AM_CPPFLAGS) 
> -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORK
>  libstat_test4_a_SOURCES = stat-test.c
>  libstat_test4_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1 
> -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test4
>  
> -test_qxl_parsing_SOURCES =   \
> - test-qxl-parsing.c  \
> - ../red-parse-qxl.c  \
> - ../memslot.c\
> - $(NULL)
> +test_qxl_parsing_LDADD = ../libserver.la $(LDADD)
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 15/15] reds_num_of_clients() -> reds_get_n_clients()

On Mon, 2016-01-18 at 16:38 +, Frediano Ziglio wrote:
> From: Jonathon Jongsma 
> 
> More consistent with glib naming conventions. Also make the function
> static since it's not used outside of this source file.

I don't know if this patch has changed slightly due to rebase, but the function
appears to already be static. So we can remove this last sentence from the
commit log.

> ---
>  server/reds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 4de1640..22ab26d 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -897,7 +897,7 @@ int reds_get_n_channels(RedsState *reds)
>  }
>  
>  
> -static int reds_num_of_clients(RedsState *reds)
> +static int reds_get_n_clients(RedsState *reds)
>  {
>  return reds ? reds->num_clients : 0;
>  }
> @@ -905,7 +905,7 @@ static int reds_num_of_clients(RedsState *reds)
>  SPICE_GNUC_VISIBLE int spice_server_get_num_clients(SpiceServer *s)
>  {
>  spice_assert(reds == s);
> -return reds_num_of_clients(reds);
> +return reds_get_n_clients(reds);
>  }
>  
>  static int secondary_channels[] = {
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 14/14] Add strings for translation

On Mon, 2016-01-18 at 10:05 +0100, Fabiano Fidêncio wrote:
> While doing the work to use GTask isntead of GSimpleAsyncResult I've
> noticed a few error strings that were not marked to be translated.
> I am not exactly sure if it was intentional or not, but I do believe
> that our error messages should be translated.
> ---
>  po/POTFILES.in   |  6 ++
>  src/channel-main.c   |  4 +++-
>  src/channel-port.c   |  4 +++-
>  src/channel-usbredir.c   |  4 ++--
>  src/spice-channel.c  |  2 +-
>  src/spice-pulse.c|  8 +---
>  src/usb-acl-helper.c | 10 ++
>  src/usb-device-manager.c |  4 ++--
>  src/vmcstream.c  |  4 +++-
>  src/win-usb-driver-install.c | 16 +---
>  10 files changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index ad12609..7c79541 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -1,8 +1,14 @@
> +src/channel-main.c
> +src/channel-port.c
>  src/channel-usbredir.c
>  src/desktop-integration.c
>  src/spice-channel.c
>  src/spice-cmdline.c
>  src/spice-option.c
> +src/spice-pulse.c
> +src/usb-acl-helper.c
>  src/usb-device-manager.c
>  src/usb-device-widget.c
>  src/usbutil.c
> +src/vmcstream.c
> +src/win-usb-driver-install.c
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 6c0f238..40d5cff 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -21,6 +21,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include "glib-compat.h"
>  #include "spice-client.h"
>  #include "spice-common.h"
> @@ -3143,7 +3145,7 @@ void spice_main_file_copy_async(SpiceMainChannel
> *channel,
>  spice_main_file_copy_async,
>  SPICE_CLIENT_ERROR,
>  SPICE_CLIENT_ERROR_FAILED,
> -"The agent is not connected");
> +_("The agent is not connected"));
>  return;
>  }
>  
> diff --git a/src/channel-port.c b/src/channel-port.c
> index 20ee2fa..86194ab 100644
> --- a/src/channel-port.c
> +++ b/src/channel-port.c
> @@ -23,6 +23,8 @@
>  #include "spice-marshal.h"
>  #include "glib-compat.h"
>  
> +#include 
> +
>  /**
>   * SECTION:channel-port
>   * @short_description: private communication channel
> @@ -295,7 +297,7 @@ void spice_port_write_async(SpicePortChannel *self,
>  g_task_report_new_error(self, callback,
>  user_data, spice_port_write_async,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"The port is not opened");
> +_("The port is not opened"));
>  return;
>  }
>  
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 4ddf7bf..824bc43 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -344,14 +344,14 @@ void spice_usbredir_channel_connect_device_async(
>  if (!priv->host) {
>  g_task_return_new_error(task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"Error libusb context not set");
> +_("Error libusb context not set"));
>  goto done;
>  }
>  
>  if (priv->state != STATE_DISCONNECTED) {
>  g_task_return_new_error(task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"Error channel is busy");
> +_("Error channel is busy"));
>  goto done;
>  }
>  
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index dd32818..246609a 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -2979,7 +2979,7 @@ void spice_channel_flush_async(SpiceChannel *self,
> GCancellable *cancellable,
>  g_task_report_new_error(self, callback, user_data,
>  spice_channel_flush_async,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"The channel is not ready yet");
> +_("The channel is not ready yet"));
>  return;
>  }
>  
> diff --git a/src/spice-pulse.c b/src/spice-pulse.c
> index 60a037c..5c3cfb7 100644
> --- a/src/spice-pulse.c
> +++ b/src/spice-pulse.c
> @@ -28,6 +28,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #define SPICE_PULSE_GET_PRIVATE(obj)  \
>  (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_PULSE, SpicePulsePrivate))
>  
> @@ -995,7 +997,7 @@ static void complete_task(SpicePulse *pulse, struct
> async_task *task, const gcha
>  g_task_return_new_error(task->task,
>  SPICE_CLIENT_ERROR,
>  SPICE_CLIENT_ERROR_FAILED,
> -"restore-info failed due %s",
> +_("restore-info failed due %s"),


As long as you're making a change here, it might be good to change this to "due
to %s" instead of "due %s"? O

Re: [Spice-devel] [spice-gtk PATCH v2] gstaudio: set output parameter to NULL on error

Acked-by: Jonathon Jongsma 

On Mon, 2016-01-18 at 15:09 +0100, Victor Toso wrote:
> This is not really triggered in the current code but this is usually
> expected in case of errors; Also, the same function on record side
> already does this.
> ---
>  src/spice-gstaudio.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> index 096fea4..2759c2b 100644
> --- a/src/spice-gstaudio.c
> +++ b/src/spice-gstaudio.c
> @@ -600,6 +600,10 @@ static gboolean
> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>  G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async),
> FALSE);
>  
>  if (g_simple_async_result_propagate_error(simple, error)) {
> +/* set out args that should have new alloc'ed memory to NULL */
> +if (volume != NULL) {
> +*volume = NULL;
> +}
>  return FALSE;
>  }
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] Glib loop and scary corruption

Hi,
I spend some time investigating in
http://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=refactory&id=48d5c2e74b6a866df2562c1aab8a1a3803bc788b,
one of the last problems of glib loop.
Beside I though that this patch also fix the symmetry between creation and
destroying (stream is initialized during creation so usually is a good idea to
deinitialize during destroying).
Looking at the backtrace you can see that stream is inside a event handler
after being freed by red_channel_client_disconnect called trying to push items
(red_channel_client_push).
My first idea was that glib called the event handler event after the watch was
freed. This proved false after some testing.
So I decided to checkout more or less same version and look at the code.
Code was failing trying to retrieve header for a message, the push was called
from red_channel_client_push handling a message of type SPICE_MSGC_ACK.
But messages are read and handled in a loop in red_peer_handle_incoming which
is called by a single watcher!
So what's happen if:
- SPICE_MSGC_ACK is read
- handler is called with SPICE_MSGC_ACK
- handler try to push messages
- write fails as connection was closed
- red_channel_client_disconnect is called
- handler return to red_peer_handle_incoming
- red_peer_handle_incoming calls red_peer_receive to read next message header
(NOTE: error handler in red_peer_handle_incoming is called only if message is
not handled)
- red_peer_receive crash as rcc->stream is NULL!

If you add a red_channel_client_disconnect after red_channel_client_push in
red_channel_client_handle_message you can see the same problem.
At this point. Why with Glib loop this happens more often? The reason is that
the push is not called at last step of iteration so there is more probability
to find items in red_channel_client_push (or at least the statistics on this
change).

So to sum up:
- this issue is not a regression introduced by glib code;
- the only thing left to check for glib is this statistical stuff (already at
good point for Jonathon work).

I'll post the patch based on master.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 11/14] vmcstream: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:47AM +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..d9b62a0 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");
>  
> -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);
>  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_inpu

Re: [Spice-devel] [PATCH 09/14] usb-acl-helper: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:45AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/usb-acl-helper.c | 76 
> 
>  1 file changed, 35 insertions(+), 41 deletions(-)
> 
> diff --git a/src/usb-acl-helper.c b/src/usb-acl-helper.c
> index 6a49627..17e2b3c 100644
> --- a/src/usb-acl-helper.c
> +++ b/src/usb-acl-helper.c
> @@ -35,10 +35,9 @@
>  (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_USB_ACL_HELPER, 
> SpiceUsbAclHelperPrivate))
>  
>  struct _SpiceUsbAclHelperPrivate {
> -GSimpleAsyncResult *result;
> +GTask *task;
>  GIOChannel *in_ch;
>  GIOChannel *out_ch;
> -GCancellable *cancellable;
>  gulong cancellable_id;
>  };
>  
> @@ -53,11 +52,11 @@ static void 
> spice_usb_acl_helper_cleanup(SpiceUsbAclHelper *self)
>  {
>  SpiceUsbAclHelperPrivate *priv = self->priv;
>  
> -g_cancellable_disconnect(priv->cancellable, priv->cancellable_id);
> -priv->cancellable = NULL;
> +g_cancellable_disconnect(g_task_get_cancellable(priv->task),
> + priv->cancellable_id);
>  priv->cancellable_id = 0;
>  
> -g_clear_object(&priv->result);
> +g_clear_object(&priv->task);
>  
>  if (priv->in_ch) {
>  g_io_channel_unref(priv->in_ch);
> @@ -90,9 +89,9 @@ static void 
> spice_usb_acl_helper_class_init(SpiceUsbAclHelperClass *klass)
>  /* -- */
>  /* callbacks  */
>  
> -static void async_result_set_cancelled(GSimpleAsyncResult *result)
> +static void async_result_set_cancelled(GTask *task)
>  {
> -g_simple_async_result_set_error(result,
> +g_task_return_new_error(task,
>  G_IO_ERROR, G_IO_ERROR_CANCELLED,
>  "Setting USB device node ACL cancelled");
>  }
> @@ -105,12 +104,13 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  SpiceUsbAclHelperPrivate *priv = self->priv;
>  gboolean success = FALSE;
>  GError *err = NULL;
> +GCancellable *cancellable;
>  GIOStatus status;
>  gchar *string;
>  gsize size;
>  
>  /* Check that we've not been cancelled */
> -if (priv->result == NULL)
> +if (priv->task == NULL)
>  goto done;
>  
>  g_return_val_if_fail(channel == priv->out_ch, FALSE);
> @@ -121,10 +121,11 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  string[strlen(string) - 1] = 0;
>  if (!strcmp(string, "SUCCESS")) {
>  success = TRUE;
> +g_task_return_boolean(priv->task, TRUE);
>  } else if (!strcmp(string, "CANCELED")) {
> -async_result_set_cancelled(priv->result);
> +async_result_set_cancelled(priv->task);
>  } else {
> -g_simple_async_result_set_error(priv->result,
> +g_task_return_new_error(priv->task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>  "Error setting USB device node ACL: '%s'",
>  string);
> @@ -132,10 +133,10 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  g_free(string);
>  break;
>  case G_IO_STATUS_ERROR:
> -g_simple_async_result_take_error(priv->result, err);
> +g_task_return_error(priv->task, err);
>  break;
>  case G_IO_STATUS_EOF:
> -g_simple_async_result_set_error(priv->result,
> +g_task_return_new_error(priv->task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>  "Unexpected EOF reading from acl helper stdout");
>  break;
> @@ -143,16 +144,16 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  return TRUE; /* Wait for more input */
>  }
>  
> -g_cancellable_disconnect(priv->cancellable, priv->cancellable_id);
> -priv->cancellable = NULL;
> +cancellable = g_task_get_cancellable(priv->task);
> +g_cancellable_disconnect(cancellable, priv->cancellable_id);
> +cancellable = NULL;

I forgot to mention in my previous mail that setting cancellable to NULL
is probably not needed.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 09/14] usb-acl-helper: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:45AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/usb-acl-helper.c | 76 
> 
>  1 file changed, 35 insertions(+), 41 deletions(-)
> 
> diff --git a/src/usb-acl-helper.c b/src/usb-acl-helper.c
> index 6a49627..17e2b3c 100644
> --- a/src/usb-acl-helper.c
> +++ b/src/usb-acl-helper.c
> @@ -35,10 +35,9 @@
>  (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_USB_ACL_HELPER, 
> SpiceUsbAclHelperPrivate))
>  
>  struct _SpiceUsbAclHelperPrivate {
> -GSimpleAsyncResult *result;
> +GTask *task;
>  GIOChannel *in_ch;
>  GIOChannel *out_ch;
> -GCancellable *cancellable;
>  gulong cancellable_id;
>  };
>  
> @@ -53,11 +52,11 @@ static void 
> spice_usb_acl_helper_cleanup(SpiceUsbAclHelper *self)
>  {
>  SpiceUsbAclHelperPrivate *priv = self->priv;
>  
> -g_cancellable_disconnect(priv->cancellable, priv->cancellable_id);
> -priv->cancellable = NULL;
> +g_cancellable_disconnect(g_task_get_cancellable(priv->task),
> + priv->cancellable_id);
>  priv->cancellable_id = 0;
>  
> -g_clear_object(&priv->result);
> +g_clear_object(&priv->task);
>  
>  if (priv->in_ch) {
>  g_io_channel_unref(priv->in_ch);
> @@ -90,9 +89,9 @@ static void 
> spice_usb_acl_helper_class_init(SpiceUsbAclHelperClass *klass)
>  /* -- */
>  /* callbacks  */
>  
> -static void async_result_set_cancelled(GSimpleAsyncResult *result)
> +static void async_result_set_cancelled(GTask *task)
>  {
> -g_simple_async_result_set_error(result,
> +g_task_return_new_error(task,
>  G_IO_ERROR, G_IO_ERROR_CANCELLED,
>  "Setting USB device node ACL cancelled");
>  }
> @@ -105,12 +104,13 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  SpiceUsbAclHelperPrivate *priv = self->priv;
>  gboolean success = FALSE;
>  GError *err = NULL;
> +GCancellable *cancellable;
>  GIOStatus status;
>  gchar *string;
>  gsize size;
>  
>  /* Check that we've not been cancelled */
> -if (priv->result == NULL)
> +if (priv->task == NULL)
>  goto done;
>  
>  g_return_val_if_fail(channel == priv->out_ch, FALSE);
> @@ -121,10 +121,11 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  string[strlen(string) - 1] = 0;
>  if (!strcmp(string, "SUCCESS")) {
>  success = TRUE;
> +g_task_return_boolean(priv->task, TRUE);
>  } else if (!strcmp(string, "CANCELED")) {
> -async_result_set_cancelled(priv->result);
> +async_result_set_cancelled(priv->task);
>  } else {
> -g_simple_async_result_set_error(priv->result,
> +g_task_return_new_error(priv->task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>  "Error setting USB device node ACL: '%s'",
>  string);
> @@ -132,10 +133,10 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  g_free(string);
>  break;
>  case G_IO_STATUS_ERROR:
> -g_simple_async_result_take_error(priv->result, err);
> +g_task_return_error(priv->task, err);
>  break;
>  case G_IO_STATUS_EOF:
> -g_simple_async_result_set_error(priv->result,
> +g_task_return_new_error(priv->task,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>  "Unexpected EOF reading from acl helper stdout");
>  break;
> @@ -143,16 +144,16 @@ static gboolean cb_out_watch(GIOChannel*channel,
>  return TRUE; /* Wait for more input */
>  }
>  
> -g_cancellable_disconnect(priv->cancellable, priv->cancellable_id);
> -priv->cancellable = NULL;
> +cancellable = g_task_get_cancellable(priv->task);
> +g_cancellable_disconnect(cancellable, priv->cancellable_id);
> +cancellable = NULL;
>  priv->cancellable_id = 0;
>  
> -g_simple_async_result_complete_in_idle(priv->result);
> -g_clear_object(&priv->result);
> -
>  if (!success)
>  spice_usb_acl_helper_cleanup(self);
>  
> +g_clear_object(&priv->task);
> +
>  done:
>  g_object_unref(self);
>  return FALSE;
> @@ -193,7 +194,7 @@ void spice_usb_acl_helper_open_acl(SpiceUsbAclHelper 
> *self,
>  g_return_if_fail(SPICE_IS_USB_ACL_HELPER(self));
>  
>  SpiceUsbAclHelperPrivate *priv = self->priv;
> -GSimpleAsyncResult *result;
> +GTask *task;
>  GError *err = NULL;
>  GIOStatus status;
>  GPid helper_pid;
> @@ -202,25 +203,24 @@ void spice_usb_acl_helper_open_acl(SpiceUsbAclHelper 
> *self,
>  g

[Spice-devel] [PATCH 09/15] Change reds_on_main_agent_start() to take RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index b5268fb..0ab3dec 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -891,7 +891,7 @@ static int main_channel_handle_parsed(RedChannelClient 
*rcc, uint32_t size, uint
 return FALSE;
 }
 tokens = (SpiceMsgcMainAgentStart *)message;
-reds_on_main_agent_start(mcc, tokens->num_tokens);
+reds_on_main_agent_start(reds, mcc, tokens->num_tokens);
 break;
 }
 case SPICE_MSGC_MAIN_AGENT_DATA: {
diff --git a/server/reds.c b/server/reds.c
index bd905e2..db1f718 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -944,7 +944,7 @@ void reds_fill_channels(RedsState *reds, SpiceMsgChannels 
*channels_info)
 }
 }
 
-void reds_on_main_agent_start(MainChannelClient *mcc, uint32_t num_tokens)
+void reds_on_main_agent_start(RedsState *reds, MainChannelClient *mcc, 
uint32_t num_tokens)
 {
 SpiceCharDeviceState *dev_state = reds->agent_state.base;
 RedChannelClient *rcc;
diff --git a/server/reds.h b/server/reds.h
index fbb616d..4033724 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -94,7 +94,7 @@ void reds_update_stat_value(uint32_t value);
 
 /* callbacks from main channel messages */
 
-void reds_on_main_agent_start(MainChannelClient *mcc, uint32_t num_tokens);
+void reds_on_main_agent_start(RedsState *reds, MainChannelClient *mcc, 
uint32_t num_tokens);
 void reds_on_main_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens);
 uint8_t *reds_get_agent_data_buffer(MainChannelClient *mcc, size_t size);
 void reds_release_agent_data_buffer(uint8_t *buf);
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 08/15] Change reds_fill_channels() to take a RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index 1b774f3..b5268fb 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -349,7 +349,7 @@ static void main_channel_marshall_channels(RedChannelClient 
*rcc,
 red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST, item);
 channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels)
 + reds_num_of_channels(reds) * 
sizeof(SpiceChannelId));
-reds_fill_channels(channels_info);
+reds_fill_channels(reds, channels_info);
 spice_marshall_msg_main_channels_list(m, channels_info);
 free(channels_info);
 }
diff --git a/server/reds.c b/server/reds.c
index 0169fac..bd905e2 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -922,7 +922,7 @@ static int channel_is_secondary(RedChannel *channel)
 return FALSE;
 }
 
-void reds_fill_channels(SpiceMsgChannels *channels_info)
+void reds_fill_channels(RedsState *reds, SpiceMsgChannels *channels_info)
 {
 RingItem *now;
 int used_channels = 0;
diff --git a/server/reds.h b/server/reds.h
index aaa9477..fbb616d 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -86,7 +86,7 @@ void reds_client_disconnect(RedsState *reds, RedClient 
*client);
 // Temporary (?) for splitting main channel
 typedef struct MainMigrateData MainMigrateData;
 void reds_marshall_migrate_data(SpiceMarshaller *m);
-void reds_fill_channels(SpiceMsgChannels *channels_info);
+void reds_fill_channels(RedsState *reds, SpiceMsgChannels *channels_info);
 int reds_num_of_channels(RedsState *reds);
 #ifdef RED_STATISTICS
 void reds_update_stat_value(uint32_t value);
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 04/15] Change vdi_port_read_buf_unref() to take RedsState arg

From: Jonathon Jongsma 

---
 server/reds.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 928cb83..4e0caba 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -214,7 +214,7 @@ static void reds_send_mm_time(void);
 
 static VDIReadBuf *vdi_port_read_buf_get(RedsState *reds);
 static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf);
-static void vdi_port_read_buf_unref(VDIReadBuf *buf);
+static void vdi_port_read_buf_unref(RedsState *reds, VDIReadBuf *buf);
 
 static ChannelSecurityOptions *channels_security = NULL;
 static int default_channel_security =
@@ -440,7 +440,7 @@ static void reds_reset_vdp(RedsState *reds)
 state->receive_len = sizeof(state->vdi_chunk_header);
 state->message_receive_len = 0;
 if (state->current_read_buf) {
-vdi_port_read_buf_unref(state->current_read_buf);
+vdi_port_read_buf_unref(reds, state->current_read_buf);
 state->current_read_buf = NULL;
 }
 /* Reset read filter to start with clean state when the agent reconnects */
@@ -658,7 +658,7 @@ static void vdi_port_read_buf_release(uint8_t *data, void 
*opaque)
 {
 VDIReadBuf *buf = (VDIReadBuf *)opaque;
 
-vdi_port_read_buf_unref(buf);
+vdi_port_read_buf_unref(reds, buf);
 }
 
 /* returns TRUE if the buffer can be forwarded */
@@ -713,7 +713,9 @@ static VDIReadBuf* vdi_port_read_buf_ref(VDIReadBuf *buf)
 return buf;
 }
 
-static void vdi_port_read_buf_unref(VDIReadBuf *buf)
+/* FIXME: refactor so that unreffing the VDIReadBuf doesn't require accessing
+ * RedsState? */
+static void vdi_port_read_buf_unref(RedsState *reds, VDIReadBuf *buf)
 {
 if (!--buf->refs) {
 ring_add(&reds->agent_state.read_bufs, &buf->link);
@@ -790,7 +792,7 @@ static SpiceCharDeviceMsgToClient 
*vdi_port_read_one_msg_from_device(SpiceCharDe
 if (vdi_port_read_buf_process(reds, state->vdi_chunk_header.port, 
dispatch_buf)) {
 return dispatch_buf;
 } else {
-vdi_port_read_buf_unref(dispatch_buf);
+vdi_port_read_buf_unref(reds, dispatch_buf);
 }
 } /* END switch */
 } /* END while */
@@ -806,7 +808,8 @@ static SpiceCharDeviceMsgToClient 
*vdi_port_ref_msg_to_client(SpiceCharDeviceMsg
 static void vdi_port_unref_msg_to_client(SpiceCharDeviceMsgToClient *msg,
   void *opaque)
 {
-vdi_port_read_buf_unref(msg);
+RedsState *reds = opaque;
+vdi_port_read_buf_unref(reds, msg);
 }
 
 /* after calling this, we unref the message, and the ref is in the instance 
side */
@@ -1163,7 +1166,7 @@ void reds_on_main_channel_migrate(MainChannelClient *mcc)
 vdi_port_read_buf_release,
 read_buf);
 } else {
-vdi_port_read_buf_unref(read_buf);
+vdi_port_read_buf_unref(reds, read_buf);
 }
 
 spice_assert(agent_state->receive_len);
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 13/15] Change red_on_main_agent_data() to take RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 6 +++---
 server/reds.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index 3315d6c..a30ba1e 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -895,7 +895,7 @@ static int main_channel_handle_parsed(RedChannelClient 
*rcc, uint32_t size, uint
 break;
 }
 case SPICE_MSGC_MAIN_AGENT_DATA: {
-reds_on_main_agent_data(mcc, message, size);
+reds_on_main_agent_data(reds, mcc, message, size);
 break;
 }
 case SPICE_MSGC_MAIN_AGENT_TOKEN: {
diff --git a/server/reds.c b/server/reds.c
index 5c038ca..37d0393 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1050,7 +1050,7 @@ static void reds_client_monitors_config_cleanup(RedsState 
*reds)
 cmc->mcc = NULL;
 }
 
-static void reds_on_main_agent_monitors_config(
+static void reds_on_main_agent_monitors_config(RedsState *reds,
 MainChannelClient *mcc, void *message, size_t size)
 {
 VDAgentMessage *msg_header;
@@ -1075,7 +1075,7 @@ static void reds_on_main_agent_monitors_config(
 reds_client_monitors_config_cleanup(reds);
 }
 
-void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t 
size)
+void reds_on_main_agent_data(RedsState *reds, MainChannelClient *mcc, void 
*message, size_t size)
 {
 VDIPortState *dev_state = &reds->agent_state;
 VDIChunkHeader *header;
@@ -1089,7 +1089,7 @@ void reds_on_main_agent_data(MainChannelClient *mcc, void 
*message, size_t size)
 case AGENT_MSG_FILTER_DISCARD:
 return;
 case AGENT_MSG_FILTER_MONITORS_CONFIG:
-reds_on_main_agent_monitors_config(mcc, message, size);
+reds_on_main_agent_monitors_config(reds, mcc, message, size);
 return;
 case AGENT_MSG_FILTER_PROTO_ERROR:
 red_channel_client_shutdown(main_channel_client_get_base(mcc));
diff --git a/server/reds.h b/server/reds.h
index 0294333..9672c5e 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -98,7 +98,7 @@ void reds_on_main_agent_start(RedsState *reds, 
MainChannelClient *mcc, uint32_t
 void reds_on_main_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens);
 uint8_t *reds_get_agent_data_buffer(RedsState *reds, MainChannelClient *mcc, 
size_t size);
 void reds_release_agent_data_buffer(RedsState *reds, uint8_t *buf);
-void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t 
size);
+void reds_on_main_agent_data(RedsState *reds, MainChannelClient *mcc, void 
*message, size_t size);
 void reds_on_main_migrate_connected(int seamless); //should be called when all 
the clients
// are connected to the 
target
 int reds_handle_migrate_data(MainChannelClient *mcc,
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 03/15] Change vdi_port_read_buf_get() to take RedsState arg

From: Jonathon Jongsma 

---
 server/reds.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index b347d1d..928cb83 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -212,7 +212,7 @@ static void reds_char_device_add_state(SpiceCharDeviceState 
*st);
 static void reds_char_device_remove_state(SpiceCharDeviceState *st);
 static void reds_send_mm_time(void);
 
-static VDIReadBuf *vdi_port_read_buf_get(void);
+static VDIReadBuf *vdi_port_read_buf_get(RedsState *reds);
 static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf);
 static void vdi_port_read_buf_unref(VDIReadBuf *buf);
 
@@ -690,7 +690,7 @@ static int vdi_port_read_buf_process(RedsState *reds, int 
port, VDIReadBuf *buf)
 }
 }
 
-static VDIReadBuf *vdi_port_read_buf_get(void)
+static VDIReadBuf *vdi_port_read_buf_get(RedsState *reds)
 {
 VDIPortState *state = &reds->agent_state;
 RingItem *item;
@@ -758,7 +758,7 @@ static SpiceCharDeviceMsgToClient 
*vdi_port_read_one_msg_from_device(SpiceCharDe
 state->message_receive_len = state->vdi_chunk_header.size;
 state->read_state = VDI_PORT_READ_STATE_GET_BUFF;
 case VDI_PORT_READ_STATE_GET_BUFF: {
-if (!(state->current_read_buf = vdi_port_read_buf_get())) {
+if (!(state->current_read_buf = vdi_port_read_buf_get(reds))) {
 return NULL;
 }
 state->receive_pos = state->current_read_buf->data;
@@ -1281,7 +1281,7 @@ static int reds_agent_state_restore(SpiceMigrateDataMain 
*mig_data)
 uint32_t cur_buf_size;
 
 agent_state->read_state = VDI_PORT_READ_STATE_READ_DATA;
-agent_state->current_read_buf = vdi_port_read_buf_get();
+agent_state->current_read_buf = vdi_port_read_buf_get(reds);
 spice_assert(agent_state->current_read_buf);
 partial_msg_header = (uint8_t *)mig_data + 
mig_data->agent2client.msg_header_ptr -
 sizeof(SpiceMiniDataHeader);
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 12/15] Change reds_client_monitors_config_cleanup() to take RedsState arg

From: Jonathon Jongsma 

---
 server/reds.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index b183ee1..5c038ca 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1040,7 +1040,7 @@ void reds_release_agent_data_buffer(RedsState *reds, 
uint8_t *buf)
 dev_state->recv_from_client_buf_pushed = FALSE;
 }
 
-static void reds_client_monitors_config_cleanup(void)
+static void reds_client_monitors_config_cleanup(RedsState *reds)
 {
 RedsClientMonitorsConfig *cmc = &reds->client_monitors_config;
 
@@ -1072,7 +1072,7 @@ static void reds_on_main_agent_monitors_config(
 monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + 
sizeof(*msg_header));
 spice_debug("%s: %d", __func__, monitors_config->num_of_monitors);
 red_dispatcher_client_monitors_config(monitors_config);
-reds_client_monitors_config_cleanup();
+reds_client_monitors_config_cleanup(reds);
 }
 
 void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t 
size)
@@ -3418,7 +3418,7 @@ static int do_spice_init(SpiceCoreInterface 
*core_interface)
 
 reds->mouse_mode = SPICE_MOUSE_MODE_SERVER;
 
-reds_client_monitors_config_cleanup();
+reds_client_monitors_config_cleanup(reds);
 
 reds->allow_multiple_clients = getenv(SPICE_DEBUG_ALLOW_MC_ENV) != NULL;
 if (reds->allow_multiple_clients) {
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 07/15] Change reds_num_of_clients() to take RedsState arg

From: Jonathon Jongsma 

---
 server/reds.c | 4 ++--
 server/reds.h | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 9b28a7c..0169fac 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -897,7 +897,7 @@ int reds_num_of_channels(RedsState *reds)
 }
 
 
-int reds_num_of_clients(void)
+static int reds_num_of_clients(RedsState *reds)
 {
 return reds ? reds->num_clients : 0;
 }
@@ -905,7 +905,7 @@ int reds_num_of_clients(void)
 SPICE_GNUC_VISIBLE int spice_server_get_num_clients(SpiceServer *s)
 {
 spice_assert(reds == s);
-return reds_num_of_clients();
+return reds_num_of_clients(reds);
 }
 
 static int secondary_channels[] = {
diff --git a/server/reds.h b/server/reds.h
index 65e8f69..aaa9477 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -88,7 +88,6 @@ typedef struct MainMigrateData MainMigrateData;
 void reds_marshall_migrate_data(SpiceMarshaller *m);
 void reds_fill_channels(SpiceMsgChannels *channels_info);
 int reds_num_of_channels(RedsState *reds);
-int reds_num_of_clients(void);
 #ifdef RED_STATISTICS
 void reds_update_stat_value(uint32_t value);
 #endif
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 14/15] reds_num_of_channels() -> reds_get_n_channels()

From: Jonathon Jongsma 

More consistent with glib naming conventions.
---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index a30ba1e..9f8a861 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -348,7 +348,7 @@ static void main_channel_marshall_channels(RedChannelClient 
*rcc,
 
 red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST, item);
 channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels)
-+ reds_num_of_channels(reds) * 
sizeof(SpiceChannelId));
++ reds_get_n_channels(reds) * 
sizeof(SpiceChannelId));
 reds_fill_channels(reds, channels_info);
 spice_marshall_msg_main_channels_list(m, channels_info);
 free(channels_info);
diff --git a/server/reds.c b/server/reds.c
index 37d0393..4de1640 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -891,7 +891,7 @@ void reds_handle_agent_mouse_event(RedsState *reds, const 
VDAgentMouseState *mou
 spice_char_device_write_buffer_add(reds->agent_state.base, char_dev_buf);
 }
 
-int reds_num_of_channels(RedsState *reds)
+int reds_get_n_channels(RedsState *reds)
 {
 return reds ? reds->num_of_channels : 0;
 }
diff --git a/server/reds.h b/server/reds.h
index 9672c5e..39fe291 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -87,7 +87,7 @@ void reds_client_disconnect(RedsState *reds, RedClient 
*client);
 typedef struct MainMigrateData MainMigrateData;
 void reds_marshall_migrate_data(SpiceMarshaller *m);
 void reds_fill_channels(RedsState *reds, SpiceMsgChannels *channels_info);
-int reds_num_of_channels(RedsState *reds);
+int reds_get_n_channels(RedsState *reds);
 #ifdef RED_STATISTICS
 void reds_update_stat_value(uint32_t value);
 #endif
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 01/15] Pass 'reds' as opaque data in vdi port char device

From: Jonathon Jongsma 

This allows us to access the RedsState variable non-globally without
changing the signature of the callback functions.
---
 server/reds.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/server/reds.c b/server/reds.c
index 9868928..25e9f90 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -733,6 +733,7 @@ static void vdi_port_read_buf_unref(VDIReadBuf *buf)
 static SpiceCharDeviceMsgToClient 
*vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *sin,
  void 
*opaque)
 {
+RedsState *reds = opaque;
 VDIPortState *state = &reds->agent_state;
 SpiceCharDeviceInterface *sif;
 VDIReadBuf *dispatch_buf;
@@ -2987,7 +2988,7 @@ static SpiceCharDeviceState 
*attach_to_red_agent(SpiceCharDeviceInstance *sin)
  REDS_TOKENS_TO_SEND,
  
REDS_NUM_INTERNAL_AGENT_MESSAGES,
  &char_dev_state_cbs,
- NULL);
+ reds);
 } else {
 spice_char_device_state_reset_dev_instance(state->base, sin);
 }
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 15/15] reds_num_of_clients() -> reds_get_n_clients()

From: Jonathon Jongsma 

More consistent with glib naming conventions. Also make the function
static since it's not used outside of this source file.
---
 server/reds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 4de1640..22ab26d 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -897,7 +897,7 @@ int reds_get_n_channels(RedsState *reds)
 }
 
 
-static int reds_num_of_clients(RedsState *reds)
+static int reds_get_n_clients(RedsState *reds)
 {
 return reds ? reds->num_clients : 0;
 }
@@ -905,7 +905,7 @@ static int reds_num_of_clients(RedsState *reds)
 SPICE_GNUC_VISIBLE int spice_server_get_num_clients(SpiceServer *s)
 {
 spice_assert(reds == s);
-return reds_num_of_clients(reds);
+return reds_get_n_clients(reds);
 }
 
 static int secondary_channels[] = {
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 11/15] Change reds_release_agent_data_buffer() to take RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index 31e65f0..3315d6c 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -1020,7 +1020,7 @@ static void 
main_channel_release_msg_rcv_buf(RedChannelClient *rcc,
uint8_t *msg)
 {
 if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
-reds_release_agent_data_buffer(msg);
+reds_release_agent_data_buffer(reds, msg);
 }
 }
 
diff --git a/server/reds.c b/server/reds.c
index d480ce3..b183ee1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1022,7 +1022,7 @@ uint8_t *reds_get_agent_data_buffer(RedsState *reds, 
MainChannelClient *mcc, siz
 return dev_state->recv_from_client_buf->buf + sizeof(VDIChunkHeader);
 }
 
-void reds_release_agent_data_buffer(uint8_t *buf)
+void reds_release_agent_data_buffer(RedsState *reds, uint8_t *buf)
 {
 VDIPortState *dev_state = &reds->agent_state;
 
diff --git a/server/reds.h b/server/reds.h
index 2659493..0294333 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -96,8 +96,8 @@ void reds_update_stat_value(uint32_t value);
 
 void reds_on_main_agent_start(RedsState *reds, MainChannelClient *mcc, 
uint32_t num_tokens);
 void reds_on_main_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens);
-void reds_release_agent_data_buffer(uint8_t *buf);
 uint8_t *reds_get_agent_data_buffer(RedsState *reds, MainChannelClient *mcc, 
size_t size);
+void reds_release_agent_data_buffer(RedsState *reds, uint8_t *buf);
 void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t 
size);
 void reds_on_main_migrate_connected(int seamless); //should be called when all 
the clients
// are connected to the 
target
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 10/15] Change reds_get_agent_data_buffer() to take RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index 0ab3dec..31e65f0 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -1008,7 +1008,7 @@ static uint8_t 
*main_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
 MainChannelClient *mcc = SPICE_CONTAINEROF(rcc, MainChannelClient, base);
 
 if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
-return reds_get_agent_data_buffer(mcc, size);
+return reds_get_agent_data_buffer(reds, mcc, size);
 } else {
 return main_chan->recv_buf;
 }
diff --git a/server/reds.c b/server/reds.c
index db1f718..d480ce3 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -997,7 +997,7 @@ void reds_on_main_agent_tokens(MainChannelClient *mcc, 
uint32_t num_tokens)
 num_tokens);
 }
 
-uint8_t *reds_get_agent_data_buffer(MainChannelClient *mcc, size_t size)
+uint8_t *reds_get_agent_data_buffer(RedsState *reds, MainChannelClient *mcc, 
size_t size)
 {
 VDIPortState *dev_state = &reds->agent_state;
 RedClient *client;
diff --git a/server/reds.h b/server/reds.h
index 4033724..2659493 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -96,8 +96,8 @@ void reds_update_stat_value(uint32_t value);
 
 void reds_on_main_agent_start(RedsState *reds, MainChannelClient *mcc, 
uint32_t num_tokens);
 void reds_on_main_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens);
-uint8_t *reds_get_agent_data_buffer(MainChannelClient *mcc, size_t size);
 void reds_release_agent_data_buffer(uint8_t *buf);
+uint8_t *reds_get_agent_data_buffer(RedsState *reds, MainChannelClient *mcc, 
size_t size);
 void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t 
size);
 void reds_on_main_migrate_connected(int seamless); //should be called when all 
the clients
// are connected to the 
target
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 06/15] Change reds_num_of_channels() to take RedsState arg

From: Jonathon Jongsma 

---
 server/main-channel.c | 2 +-
 server/reds.c | 2 +-
 server/reds.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index 2a9f0ce..1b774f3 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -348,7 +348,7 @@ static void main_channel_marshall_channels(RedChannelClient 
*rcc,
 
 red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST, item);
 channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels)
-+ reds_num_of_channels() * sizeof(SpiceChannelId));
++ reds_num_of_channels(reds) * 
sizeof(SpiceChannelId));
 reds_fill_channels(channels_info);
 spice_marshall_msg_main_channels_list(m, channels_info);
 free(channels_info);
diff --git a/server/reds.c b/server/reds.c
index 11f0e1a..9b28a7c 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -891,7 +891,7 @@ void reds_handle_agent_mouse_event(RedsState *reds, const 
VDAgentMouseState *mou
 spice_char_device_write_buffer_add(reds->agent_state.base, char_dev_buf);
 }
 
-int reds_num_of_channels(void)
+int reds_num_of_channels(RedsState *reds)
 {
 return reds ? reds->num_of_channels : 0;
 }
diff --git a/server/reds.h b/server/reds.h
index 76262ce..65e8f69 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -87,7 +87,7 @@ void reds_client_disconnect(RedsState *reds, RedClient 
*client);
 typedef struct MainMigrateData MainMigrateData;
 void reds_marshall_migrate_data(SpiceMarshaller *m);
 void reds_fill_channels(SpiceMsgChannels *channels_info);
-int reds_num_of_channels(void);
+int reds_num_of_channels(RedsState *reds);
 int reds_num_of_clients(void);
 #ifdef RED_STATISTICS
 void reds_update_stat_value(uint32_t value);
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 05/15] Change reds_handle_agent_mouse_event() to take RedsState arg

From: Jonathon Jongsma 

---
 server/inputs-channel.c | 6 +++---
 server/reds.c   | 5 +++--
 server/reds.h   | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/server/inputs-channel.c b/server/inputs-channel.c
index bb5b203..d13f9d7 100644
--- a/server/inputs-channel.c
+++ b/server/inputs-channel.c
@@ -396,7 +396,7 @@ static int inputs_channel_handle_parsed(RedChannelClient 
*rcc, uint32_t size, ui
 mouse_state->y = pos->y;
 mouse_state->buttons = 
RED_MOUSE_BUTTON_STATE_TO_AGENT(pos->buttons_state);
 mouse_state->display_id = pos->display_id;
-reds_handle_agent_mouse_event(mouse_state);
+reds_handle_agent_mouse_event(reds, mouse_state);
 break;
 }
 case SPICE_MSGC_INPUTS_MOUSE_PRESS: {
@@ -413,7 +413,7 @@ static int inputs_channel_handle_parsed(RedChannelClient 
*rcc, uint32_t size, ui
 
RED_MOUSE_BUTTON_STATE_TO_AGENT(mouse_press->buttons_state) |
 (dz == -1 ? VD_AGENT_UBUTTON_MASK : 0) |
 (dz == 1 ? VD_AGENT_DBUTTON_MASK : 0);
-reds_handle_agent_mouse_event(&inputs_channel->mouse_state);
+reds_handle_agent_mouse_event(reds, 
&inputs_channel->mouse_state);
 } else if (tablet) {
 SpiceTabletInterface *sif;
 sif = SPICE_CONTAINEROF(tablet->base.sif, 
SpiceTabletInterface, base);
@@ -433,7 +433,7 @@ static int inputs_channel_handle_parsed(RedChannelClient 
*rcc, uint32_t size, ui
 if (reds_get_agent_mouse() && reds_has_vdagent()) {
 inputs_channel->mouse_state.buttons =
 
RED_MOUSE_BUTTON_STATE_TO_AGENT(mouse_release->buttons_state);
-reds_handle_agent_mouse_event(&inputs_channel->mouse_state);
+reds_handle_agent_mouse_event(reds, 
&inputs_channel->mouse_state);
 } else if (tablet) {
 SpiceTabletInterface *sif;
 sif = SPICE_CONTAINEROF(tablet->base.sif, 
SpiceTabletInterface, base);
diff --git a/server/reds.c b/server/reds.c
index 4e0caba..11f0e1a 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -834,10 +834,11 @@ static void vdi_port_send_tokens_to_client(RedClient 
*client, uint32_t tokens, v
 
 static void vdi_port_on_free_self_token(void *opaque)
 {
+RedsState *reds = opaque;
 
 if (inputs_inited() && reds->pending_mouse_event) {
 spice_debug("pending mouse event");
-reds_handle_agent_mouse_event(inputs_get_mouse_state());
+reds_handle_agent_mouse_event(reds, inputs_get_mouse_state());
 }
 }
 
@@ -854,7 +855,7 @@ int reds_has_vdagent(void)
 return !!vdagent;
 }
 
-void reds_handle_agent_mouse_event(const VDAgentMouseState *mouse_state)
+void reds_handle_agent_mouse_event(RedsState *reds, const VDAgentMouseState 
*mouse_state)
 {
 SpiceCharDeviceWriteBuffer *char_dev_buf;
 VDInternalBuf *internal_buf;
diff --git a/server/reds.h b/server/reds.h
index e38a99a..76262ce 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -60,7 +60,7 @@ void reds_unregister_channel(RedsState *reds, RedChannel 
*channel);
 int reds_get_mouse_mode(RedsState *reds); // used by inputs_channel
 int reds_get_agent_mouse(void); // used by inputs_channel
 int reds_has_vdagent(void); // used by inputs channel
-void reds_handle_agent_mouse_event(const VDAgentMouseState *mouse_state); // 
used by inputs_channel
+void reds_handle_agent_mouse_event(RedsState *reds, const VDAgentMouseState 
*mouse_state); // used by inputs_channel
 
 enum {
 RED_RENDERER_INVALID,
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 02/15] Change vdi_port_read_buf_process() to take RedsState arg

From: Jonathon Jongsma 

---
 server/reds.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 25e9f90..b347d1d 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -662,7 +662,7 @@ static void vdi_port_read_buf_release(uint8_t *data, void 
*opaque)
 }
 
 /* returns TRUE if the buffer can be forwarded */
-static int vdi_port_read_buf_process(int port, VDIReadBuf *buf)
+static int vdi_port_read_buf_process(RedsState *reds, int port, VDIReadBuf 
*buf)
 {
 VDIPortState *state = &reds->agent_state;
 int res;
@@ -787,7 +787,7 @@ static SpiceCharDeviceMsgToClient 
*vdi_port_read_one_msg_from_device(SpiceCharDe
 } else {
 state->read_state = VDI_PORT_READ_STATE_GET_BUFF;
 }
-if (vdi_port_read_buf_process(state->vdi_chunk_header.port, 
dispatch_buf)) {
+if (vdi_port_read_buf_process(reds, state->vdi_chunk_header.port, 
dispatch_buf)) {
 return dispatch_buf;
 } else {
 vdi_port_read_buf_unref(dispatch_buf);
@@ -1156,7 +1156,7 @@ void reds_on_main_channel_migrate(MainChannelClient *mcc)
 !agent_state->read_filter.msg_data_to_read);
 
 read_buf->len = read_data_len;
-if (vdi_port_read_buf_process(agent_state->vdi_chunk_header.port, 
read_buf)) {
+if (vdi_port_read_buf_process(reds, 
agent_state->vdi_chunk_header.port, read_buf)) {
 main_channel_client_push_agent_data(mcc,
 read_buf->data,
 read_buf->len,
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 00/15] Backported some patches from refactory branches (18th Jan)

Many new patches try to incrementally remove global reds variable so
they add RedsState* as parameter.

All previous patches were merged.

Jonathon Jongsma (15):
  Pass 'reds' as opaque data in vdi port char device
  Change vdi_port_read_buf_process() to take RedsState arg
  Change vdi_port_read_buf_get() to take RedsState arg
  Change vdi_port_read_buf_unref() to take RedsState arg
  Change reds_handle_agent_mouse_event() to take RedsState arg
  Change reds_num_of_channels() to take RedsState arg
  Change reds_num_of_clients() to take RedsState arg
  Change reds_fill_channels() to take a RedsState arg
  Change reds_on_main_agent_start() to take RedsState arg
  Change reds_get_agent_data_buffer() to take RedsState arg
  Change reds_release_agent_data_buffer() to take RedsState arg
  Change reds_client_monitors_config_cleanup() to take RedsState arg
  Change red_on_main_agent_data() to take RedsState arg
  reds_num_of_channels() -> reds_get_n_channels()
  reds_num_of_clients() -> reds_get_n_clients()

 server/inputs-channel.c |  6 ++---
 server/main-channel.c   | 12 -
 server/reds.c   | 65 ++---
 server/reds.h   | 15 ++--
 4 files changed, 51 insertions(+), 47 deletions(-)

-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] tests: remove leaks in test-qxl-parsing

This make happy address sanitizer during make check.
Otherwise memory leak detector can keep in and make tests fails.

Signed-off-by: Frediano Ziglio 
---
 server/tests/test-qxl-parsing.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index d44289f..a3b2e6c 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -65,6 +65,8 @@ int main(int argc, char **argv)
 QXLCursor *cursor;
 QXLDataChunk *chunks[2];
 
+void *surface_mem;
+
 memset(&qxl, 0, sizeof(qxl));
 
 qxl.surface_id = 123;
@@ -75,7 +77,8 @@ int main(int argc, char **argv)
 qxl.u.surface_create.width = 128;
 qxl.u.surface_create.stride = 512;
 qxl.u.surface_create.height = 128;
-qxl.u.surface_create.data = to_physical(malloc(0x1));
+surface_mem = malloc(0x1);
+qxl.u.surface_create.data = to_physical(surface_mem);
 if (red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
 failure();
 
@@ -117,6 +120,7 @@ int main(int argc, char **argv)
 
 if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, 
to_physical(&cursor_cmd)))
 failure();
+free(red_cursor_cmd.u.set.shape.data);
 free(cursor);
 
 /* a circular list of empty chunks should not be a problems */
@@ -173,5 +177,9 @@ int main(int argc, char **argv)
 free(cursor);
 free(chunks[0]);
 
+free(mem_info.mem_slots[0]);
+free(mem_info.mem_slots);
+free(surface_mem);
+
 return exit_code;
 }
-- 
2.4.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 13/14] wocky-http-proxy: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:49AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.

Is there an upstream for wocky-http-proxy where you should send this
patch?

> ---
>  src/wocky-http-proxy.c | 47 ++-
>  1 file changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/src/wocky-http-proxy.c b/src/wocky-http-proxy.c
> index d84cd72..f079e10 100644
> --- a/src/wocky-http-proxy.c
> +++ b/src/wocky-http-proxy.c
> @@ -254,14 +254,13 @@ error:
>  
>  typedef struct
>  {
> -  GSimpleAsyncResult *simple;
> +  GTask *task;
>GIOStream *io_stream;
>gchar *buffer;
>gssize length;
>gssize offset;
>GDataInputStream *data_in;
>gboolean has_cred;
> -  GCancellable *cancellable;
>  } ConnectAsyncData;
>  
>  static void request_write_cb (GObject *source,
> @@ -282,26 +281,22 @@ free_connect_data (ConnectAsyncData *data)
>if (data->data_in != NULL)
>  g_object_unref (data->data_in);
>  
> -  if (data->cancellable != NULL)
> -g_object_unref (data->cancellable);
> -
>g_free (data);
>  }
>  
>  static void
>  complete_async_from_error (ConnectAsyncData *data, GError *error)
>  {
> -  GSimpleAsyncResult *simple = data->simple;
> +  GTask *task = data->task;
>  
>if (error == NULL)
>  g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_PROXY_FAILED,
>  "HTTP proxy server closed connection unexpectedly.");
>  
> -  g_simple_async_result_set_from_error (data->simple, error);
> +  g_task_return_error(data->task, error);
>g_error_free (error);

This should be removed

> -  g_simple_async_result_set_op_res_gpointer (simple, NULL, NULL);
> -  g_simple_async_result_complete (simple);
> -  g_object_unref (simple);
> +  g_task_return_pointer(task, NULL, NULL);

I don't think you need this g_task_return_pointer(), you already
returned an error.

> +  g_object_unref (task);
>  }
>  
>  static void
> @@ -312,7 +307,7 @@ do_write (GAsyncReadyCallback callback, ConnectAsyncData 
> *data)
>g_output_stream_write_async (out,
>data->buffer + data->offset,
>data->length - data->offset,
> -  G_PRIORITY_DEFAULT, data->cancellable,
> +  G_PRIORITY_DEFAULT, g_task_get_cancellable(data->task),
>callback, data);
>  }
>  
> @@ -357,24 +352,22 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>  GAsyncReadyCallback callback,
>  gpointer user_data)
>  {
> -  GSimpleAsyncResult *simple;
> +  GTask *task;
>ConnectAsyncData *data;
>  
> -  simple = g_simple_async_result_new (G_OBJECT (proxy),
> -  callback, user_data,
> -  wocky_http_proxy_connect_async);
> +  task = g_task_new (proxy,
> + cancellable,
> + callback,
> + user_data);
>  
>data = g_new0 (ConnectAsyncData, 1);
> -  if (cancellable != NULL)
> -data->cancellable = g_object_ref (cancellable);
> -  data->simple = simple;
> +  data->task = task;
>  
>data->buffer = create_request (proxy_address, &data->has_cred);
>data->length = strlen (data->buffer);
>data->offset = 0;
>  
> -  g_simple_async_result_set_op_res_gpointer (simple, data,
> - (GDestroyNotify) 
> free_connect_data);
> +  g_task_return_pointer (task, data, (GDestroyNotify) free_connect_data);

Ah, I don't think the initial code meant g_task_return_pointer() here
(there is no call to g_simple_async_result_complete_* in this function).
It's rather a slight abuse of GSimpleAsyncResult API in order to get
g_task_set_task_data functionality. Or maybe this is really the value
that it wants to return to the user, but not before it's time to do that
in reply_read_cb().

>  
>if (WOCKY_IS_HTTPS_PROXY (proxy))
>  {
> @@ -435,7 +428,7 @@ request_write_cb (GObject *source,
>g_data_input_stream_read_until_async (data->data_in,
>HTTP_END_MARKER,
>G_PRIORITY_DEFAULT,
> -  data->cancellable,
> +  g_task_get_cancellable(data->task),
>reply_read_cb, data);
>  
>  }
> @@ -468,8 +461,7 @@ reply_read_cb (GObject *source,
>return;
>  }
>  
> -  g_simple_async_result_complete (data->simple);

You need a g_task_return_ here in my opinion.


Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk PATCH v2] gstaudio: set output parameter to NULL on error

hmmm, subjectprefix with v2. sorry. This is v1.

On Mon, Jan 18, 2016 at 03:09:48PM +0100, Victor Toso wrote:
> This is not really triggered in the current code but this is usually
> expected in case of errors; Also, the same function on record side
> already does this.
> ---
>  src/spice-gstaudio.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> index 096fea4..2759c2b 100644
> --- a/src/spice-gstaudio.c
> +++ b/src/spice-gstaudio.c
> @@ -600,6 +600,10 @@ static gboolean 
> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>  G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async), 
> FALSE);
>  
>  if (g_simple_async_result_propagate_error(simple, error)) {
> +/* set out args that should have new alloc'ed memory to NULL */
> +if (volume != NULL) {
> +*volume = NULL;
> +}
>  return FALSE;
>  }
>  
> -- 
> 2.5.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk PATCH v2] gstaudio: set output parameter to NULL on error

This is not really triggered in the current code but this is usually
expected in case of errors; Also, the same function on record side
already does this.
---
 src/spice-gstaudio.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
index 096fea4..2759c2b 100644
--- a/src/spice-gstaudio.c
+++ b/src/spice-gstaudio.c
@@ -600,6 +600,10 @@ static gboolean 
spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
 G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async), 
FALSE);
 
 if (g_simple_async_result_propagate_error(simple, error)) {
+/* set out args that should have new alloc'ed memory to NULL */
+if (volume != NULL) {
+*volume = NULL;
+}
 return FALSE;
 }
 
-- 
2.5.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 12/14] win-usb-driver-install: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:48AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/win-usb-driver-install.c | 86 
> +---
>  1 file changed, 41 insertions(+), 45 deletions(-)
> 
> diff --git a/src/win-usb-driver-install.c b/src/win-usb-driver-install.c
> index 54e9b14..a7c4864 100644
> --- a/src/win-usb-driver-install.c
> +++ b/src/win-usb-driver-install.c
> @@ -44,8 +44,7 @@
>  
>  struct _SpiceWinUsbDriverPrivate {
>  USBClerkReply reply;
> -GSimpleAsyncResult*result;
> -GCancellable  *cancellable;
> +GTask *task;
>  HANDLEhandle;
>  SpiceUsbDevice*device;
>  };
> @@ -144,16 +143,16 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
>  
>  if (err) {
>  g_warning("failed to read reply from usbclerk (%s)", err->message);
> -g_simple_async_result_take_error(priv->result, err);
> +g_task_return_error(priv->task, err);
>  goto failed_reply;
>  }
>  
>  if (bytes == 0) {
>  g_warning("unexpected EOF from usbclerk");
> -g_simple_async_result_set_error(priv->result,
> -SPICE_WIN_USB_DRIVER_ERROR,
> -SPICE_WIN_USB_DRIVER_ERROR_FAILED,
> -"unexpected EOF from usbclerk");
> +g_task_return_new_error(priv->task,
> +SPICE_WIN_USB_DRIVER_ERROR,
> +SPICE_WIN_USB_DRIVER_ERROR_FAILED,
> +"unexpected EOF from usbclerk");
>  goto failed_reply;
>  }
>  
> @@ -167,52 +166,53 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
>  if (priv->reply.hdr.magic != USB_CLERK_MAGIC) {
>  g_warning("usbclerk magic mismatch: mine=0x%04x  server=0x%04x",
>USB_CLERK_MAGIC, priv->reply.hdr.magic);
> -g_simple_async_result_set_error(priv->result,
> -SPICE_WIN_USB_DRIVER_ERROR,
> -SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> -"usbclerk magic mismatch");
> +g_task_return_new_error(priv->task,
> +SPICE_WIN_USB_DRIVER_ERROR,
> +SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> +"usbclerk magic mismatch");
>  goto failed_reply;
>  }
>  
>  if (priv->reply.hdr.version != USB_CLERK_VERSION) {
>  g_warning("usbclerk version mismatch: mine=0x%04x  server=0x%04x",
>USB_CLERK_VERSION, priv->reply.hdr.version);
> -g_simple_async_result_set_error(priv->result,
> -SPICE_WIN_USB_DRIVER_ERROR,
> -SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> -"usbclerk version mismatch");
> +g_task_return_new_error(priv->task,
> +SPICE_WIN_USB_DRIVER_ERROR,
> +SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> +"usbclerk version mismatch");
>  }
>  
>  if (priv->reply.hdr.type != USB_CLERK_REPLY) {
>  g_warning("usbclerk message with unexpected type %d",
>priv->reply.hdr.type);
> -g_simple_async_result_set_error(priv->result,
> -SPICE_WIN_USB_DRIVER_ERROR,
> -SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> -"usbclerk message with unexpected 
> type");
> +g_task_return_new_error(priv->task,
> +SPICE_WIN_USB_DRIVER_ERROR,
> +SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> +"usbclerk message with unexpected type");
>  goto failed_reply;
>  }
>  
>  if (priv->reply.hdr.size != bytes) {
>  g_warning("usbclerk message size mismatch: read %"G_GSSIZE_FORMAT" 
> bytes  hdr.size=%d",
>bytes, priv->reply.hdr.size);
> -g_simple_async_result_set_error(priv->result,
> -SPICE_WIN_USB_DRIVER_ERROR,
> -SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> -"usbclerk message with unexpected 
> size");
> +g_task_return_new_error(priv->task,
> +SPICE_WIN_USB_DRIVER_ERROR,
> +SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> +"usbclerk message with unexpected size");
>  goto failed_reply;
>  }
>  
>  if (priv->reply.status == 0) {
> -g_simple_async_result_set_error(priv->resul

Re: [Spice-devel] [PATCH 07/14] spice-gstaudio: Use GTask instead of GSimpleAsyncResult

Hi,

On Mon, Jan 18, 2016 at 02:51:05PM +0100, Christophe Fergeau wrote:
> On Mon, Jan 18, 2016 at 10:05:43AM +0100, Fabiano Fidêncio wrote:
> > Instead of using GSimpleAsyncResult, use the new GTask API, which is
> > much more straightforward.
> > ---
> >  src/spice-gstaudio.c | 48 ++--
> >  1 file changed, 10 insertions(+), 38 deletions(-)
> > 
> > diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> > index 096fea4..65fc173 100644
> > --- a/src/spice-gstaudio.c
> > +++ b/src/spice-gstaudio.c
> > @@ -570,16 +570,9 @@ static void 
> > spice_gstaudio_get_playback_volume_info_async(SpiceAudio *audio,
> >
> > GAsyncReadyCallback callback,
> >gpointer 
> > user_data)
> >  {
> > -GSimpleAsyncResult *simple;
> > +GTask *task = g_task_new(audio, cancellable, callback, user_data);
> >  
> > -simple = g_simple_async_result_new(G_OBJECT(audio),
> > -   callback,
> > -   user_data,
> > -   
> > spice_gstaudio_get_playback_volume_info_async);
> > -g_simple_async_result_set_check_cancellable (simple, cancellable);
> > -
> > -g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> > -g_simple_async_result_complete_in_idle(simple);
> > +g_task_return_boolean(task, TRUE);
> >  }
> >  
> >  static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio 
> > *audio,
> > @@ -594,14 +587,9 @@ static gboolean 
> > spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
> >  gboolean lmute;
> >  gdouble vol;
> >  gboolean fake_channel = FALSE;
> > -GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> > -
> > -g_return_val_if_fail(g_simple_async_result_is_valid(res,
> > -G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async), 
> > FALSE);
> > +GTask *task = G_TASK(res);
> >  
> > -if (g_simple_async_result_propagate_error(simple, error)) {
> > -return FALSE;
> > -}
> > +g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);
> 
> 
> I would keep a
> if (g_task_had_error(task)) {
> return g_task_propage_error(task, error);
> }
> 
> to avoid trying to do a lot of things when there had been an error.
> 
> >  if (p->playback.sink == NULL || p->playback.channels == 0) {
> >  SPICE_DEBUG("PlaybackChannel not created yet, force start");
> > @@ -647,7 +635,7 @@ static gboolean 
> > spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
> >  }
> >  }
> >  
> > -return g_simple_async_result_get_op_res_gboolean(simple);
> > +return g_task_propagate_boolean(task, error);
> >  }
> >  
> >  static void spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> > @@ -656,16 +644,9 @@ static void 
> > spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> >  
> > GAsyncReadyCallback callback,
> >  gpointer user_data)
> >  {
> > -GSimpleAsyncResult *simple;
> > -
> > -simple = g_simple_async_result_new(G_OBJECT(audio),
> > -   callback,
> > -   user_data,
> > -   
> > spice_gstaudio_get_record_volume_info_async);
> > -g_simple_async_result_set_check_cancellable (simple, cancellable);
> > +GTask *task = g_task_new(audio, cancellable, callback, user_data);
> >  
> > -g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> > -g_simple_async_result_complete_in_idle(simple);
> > +g_task_return_boolean(task, TRUE);
> >  }
> >  
> >  static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio 
> > *audio,
> > @@ -680,18 +661,9 @@ static gboolean 
> > spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
> >  gboolean lmute;
> >  gdouble vol;
> >  gboolean fake_channel = FALSE;
> > -GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> > +GTask *task = G_TASK(res);
> >
> > -g_return_val_if_fail(g_simple_async_result_is_valid(res,
> > -G_OBJECT(audio), spice_gstaudio_get_record_volume_info_async), 
> > FALSE);
> > -
> > -if (g_simple_async_result_propagate_error(simple, error)) {
> > -/* set out args that should have new alloc'ed memory to NULL */
> > -if (volume != NULL) {
> > -*volume = NULL;
> > -}
>
> I would keep this too, I'm a bit surprised it's only done in the record
> case and not in the playback case.

Indeed, seems that it was forgotten in the playback case.
Both calls from channel-main don't mess with the output arg if the
function return FALSE but it is recommended to set to NULL.

>
> Christophe

> 
> > -return FALSE;
> > -}
> > +

Re: [Spice-devel] [PATCH 10/14] usb-device-manager: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:46AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/usb-device-manager.c | 61 
> 
>  1 file changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index c62f56e..1840177 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -1055,15 +1055,16 @@ static void 
> spice_usb_device_manager_channel_connect_cb(
>  GObject *gobject, GAsyncResult *channel_res, gpointer user_data)
>  {
>  SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(gobject);
> -GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(user_data);
> +GTask *task = G_TASK(user_data);
>  GError *err = NULL;
>  
>  spice_usbredir_channel_connect_device_finish(channel, channel_res, &err);
> -if (err) {
> -g_simple_async_result_take_error(result, err);
> -}
> -g_simple_async_result_complete(result);
> -g_object_unref(result);
> +if (err)
> +g_task_return_error(task, err);
> +else
> +g_task_return_boolean(task, TRUE);
> +
> +g_object_unref(task);
>  }
>  
>  #ifdef G_OS_WIN32
> @@ -1229,7 +1230,7 @@ static void 
> spice_usb_device_manager_check_redir_on_connect(
>  SpiceUsbDeviceManager *self, SpiceChannel *channel)
>  {
>  SpiceUsbDeviceManagerPrivate *priv = self->priv;
> -GSimpleAsyncResult *result;
> +GTask *task;
>  SpiceUsbDevice *device;
>  libusb_device *libdev;
>  guint i;
> @@ -1254,15 +1255,19 @@ static void 
> spice_usb_device_manager_check_redir_on_connect(
>  libdev, 0) == 0) {
>  /* Note: re-uses spice_usb_device_manager_connect_device_async's
> completion handling code! */
> -result = g_simple_async_result_new(G_OBJECT(self),
> -   spice_usb_device_manager_auto_connect_cb,
> -   spice_usb_device_ref(device),
> -   
> spice_usb_device_manager_connect_device_async);
> +task = g_task_new(self,
> +  NULL,
> +  spice_usb_device_manager_auto_connect_cb,
> +  spice_usb_device_ref(device));
> +
> +g_task_set_source_tag(task,
> +  spice_usb_device_manager_connect_device_async);
> +

I don't think this is used.

Acked-by: Christophe Fergeau 

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 08/14] spice-pulse: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:44AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/spice-pulse.c | 96 
> +++
>  1 file changed, 33 insertions(+), 63 deletions(-)
> 
> diff --git a/src/spice-pulse.c b/src/spice-pulse.c
> index 22db893..60a037c 100644
> --- a/src/spice-pulse.c
> +++ b/src/spice-pulse.c
> @@ -34,13 +34,12 @@
>  struct async_task {
>  SpicePulse *pulse;
>  SpiceMainChannel   *main_channel;
> -GSimpleAsyncResult *res;
> +GTask  *task;
>  GAsyncReadyCallbackcallback;
>  gpointer   user_data;
>  gboolean   is_playback;
>  pa_operation   *pa_op;
>  gulong cancel_id;
> -GCancellable   *cancellable;
>  };
>  
>  struct stream {
> @@ -941,8 +940,8 @@ static gboolean free_async_task(gpointer user_data)
>  g_object_unref(task->pulse);
>  }
>  
> -if (task->res)
> -g_object_unref(task->res);
> +if (task->task)
> +g_object_unref(task->task);
>  
>  if (task->main_channel)
>  g_object_unref(task->main_channel);
> @@ -950,10 +949,9 @@ static gboolean free_async_task(gpointer user_data)
>  if (task->pa_op != NULL)
>  pa_operation_unref(task->pa_op);
>  
> -if (task->cancel_id != 0) {
> -g_cancellable_disconnect(task->cancellable, task->cancel_id);
> -g_clear_object(&task->cancellable);
> -}
> +if (task->cancel_id != 0)
> +g_cancellable_disconnect(g_task_get_cancellable(task->task),
> + task->cancel_id);

You need to move the g_object_unref(task->task) after this.

>  
>  g_free(task);
>  return G_SOURCE_REMOVE;
> @@ -981,14 +979,6 @@ static void cancel_task(GCancellable *cancellable, 
> gpointer user_data)
>  task->pulse->priv->pending_restore_task = NULL;
>  }
>  
> -#if !GLIB_CHECK_VERSION(2,32,0)
> -/* g_simple_async_result_set_check_cancellable is not present. Set an 
> error
> - * in the GSimpleAsyncResult in case of _finish functions is called */
> -g_simple_async_result_set_error(task->res,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_FAILED,
> -"Operation was cancelled");
> -#endif
>  /* FIXME: https://bugzilla.gnome.org/show_bug.cgi?id=705395
>   * Free the memory in idle */
>  g_idle_add(free_async_task, task);
> @@ -1001,27 +991,23 @@ static void complete_task(SpicePulse *pulse, struct 
> async_task *task, const gcha
>  
>  /* If we do have any err_msg, we failed */
>  if (err_msg != NULL) {
> -g_simple_async_result_set_op_res_gboolean(task->res, FALSE);
> -g_simple_async_result_set_error(task->res,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_FAILED,
> -"restore-info failed due %s",
> -err_msg);
> +g_task_return_boolean(task->task, FALSE);

This can be removed.

> +g_task_return_new_error(task->task,
> +SPICE_CLIENT_ERROR,
> +SPICE_CLIENT_ERROR_FAILED,
> +"restore-info failed due %s",
> +err_msg);
>  /* Volume-info does not change if stream is not found */
>  } else if ((task->is_playback == TRUE && p->playback.info_updated == 
> FALSE) ||
> (task->is_playback == FALSE && p->record.info_updated == 
> FALSE)) {
> -g_simple_async_result_set_op_res_gboolean(task->res, FALSE);
> -g_simple_async_result_set_error(task->res,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_FAILED,
> -"Stream not found by pulse");
> +g_task_return_boolean(task->task, FALSE);

This can be removed too.

> +g_task_return_new_error(task->task,
> +SPICE_CLIENT_ERROR,
> +SPICE_CLIENT_ERROR_FAILED,
> +"Stream not found by pulse");
>  } else {
> -g_simple_async_result_set_op_res_gboolean(task->res, TRUE);
> +g_task_return_boolean(task->task, TRUE);
>  }
> -
> -/* As all async calls to PulseAudio are done with glib mainloop, it is
> - * safe to complete the operation synchronously here. */
> -g_simple_async_result_complete(task->res);
>  }
>  
>  static void spice_pulse_complete_async_task(struct async_task *task, const 
> gchar *err_msg)
> @@ -1157,19 +1143,13 @@ static void pulse_stream_restore_info_async(gboolean

Re: [Spice-devel] [PATCH 07/14] spice-gstaudio: Use GTask instead of GSimpleAsyncResult

On Mon, Jan 18, 2016 at 10:05:43AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/spice-gstaudio.c | 48 ++--
>  1 file changed, 10 insertions(+), 38 deletions(-)
> 
> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> index 096fea4..65fc173 100644
> --- a/src/spice-gstaudio.c
> +++ b/src/spice-gstaudio.c
> @@ -570,16 +570,9 @@ static void 
> spice_gstaudio_get_playback_volume_info_async(SpiceAudio *audio,
>
> GAsyncReadyCallback callback,
>gpointer user_data)
>  {
> -GSimpleAsyncResult *simple;
> +GTask *task = g_task_new(audio, cancellable, callback, user_data);
>  
> -simple = g_simple_async_result_new(G_OBJECT(audio),
> -   callback,
> -   user_data,
> -   
> spice_gstaudio_get_playback_volume_info_async);
> -g_simple_async_result_set_check_cancellable (simple, cancellable);
> -
> -g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> -g_simple_async_result_complete_in_idle(simple);
> +g_task_return_boolean(task, TRUE);
>  }
>  
>  static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio 
> *audio,
> @@ -594,14 +587,9 @@ static gboolean 
> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>  gboolean lmute;
>  gdouble vol;
>  gboolean fake_channel = FALSE;
> -GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> -
> -g_return_val_if_fail(g_simple_async_result_is_valid(res,
> -G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async), 
> FALSE);
> +GTask *task = G_TASK(res);
>  
> -if (g_simple_async_result_propagate_error(simple, error)) {
> -return FALSE;
> -}
> +g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);


I would keep a
if (g_task_had_error(task)) {
return g_task_propage_error(task, error);
}

to avoid trying to do a lot of things when there had been an error.

>  if (p->playback.sink == NULL || p->playback.channels == 0) {
>  SPICE_DEBUG("PlaybackChannel not created yet, force start");
> @@ -647,7 +635,7 @@ static gboolean 
> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>  }
>  }
>  
> -return g_simple_async_result_get_op_res_gboolean(simple);
> +return g_task_propagate_boolean(task, error);
>  }
>  
>  static void spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> @@ -656,16 +644,9 @@ static void 
> spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
>  GAsyncReadyCallback 
> callback,
>  gpointer user_data)
>  {
> -GSimpleAsyncResult *simple;
> -
> -simple = g_simple_async_result_new(G_OBJECT(audio),
> -   callback,
> -   user_data,
> -   
> spice_gstaudio_get_record_volume_info_async);
> -g_simple_async_result_set_check_cancellable (simple, cancellable);
> +GTask *task = g_task_new(audio, cancellable, callback, user_data);
>  
> -g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> -g_simple_async_result_complete_in_idle(simple);
> +g_task_return_boolean(task, TRUE);
>  }
>  
>  static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio 
> *audio,
> @@ -680,18 +661,9 @@ static gboolean 
> spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
>  gboolean lmute;
>  gdouble vol;
>  gboolean fake_channel = FALSE;
> -GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> +GTask *task = G_TASK(res);
>  
> -g_return_val_if_fail(g_simple_async_result_is_valid(res,
> -G_OBJECT(audio), spice_gstaudio_get_record_volume_info_async), 
> FALSE);
> -
> -if (g_simple_async_result_propagate_error(simple, error)) {
> -/* set out args that should have new alloc'ed memory to NULL */
> -if (volume != NULL) {
> -*volume = NULL;
> -}

I would keep this too, I'm a bit surprised it's only done in the record
case and not in the playback case.

Christophe

> -return FALSE;
> -}
> +g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);
>  
>  if (p->record.src == NULL || p->record.channels == 0) {
>  SPICE_DEBUG("RecordChannel not created yet, force start");
> @@ -737,5 +709,5 @@ static gboolean 
> spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
>  }
>  }
>  
> -return g_simple_async_result_get_op_res_gboolean(simple);
> +return g_task_propagate_boolean(task, error);
>  }
> -- 
> 2.5.0
> 
> __

Re: [Spice-devel] [PATCH 06/14] spice-channel: Use GTask instead of GSimpleAsyncResult

Acked-by: Christophe Fergeau 

On Mon, Jan 18, 2016 at 10:05:42AM +0100, Fabiano Fidêncio wrote:
> Instead of using GSimpleAsyncResult, use the new GTask API, which is
> much more straightforward.
> ---
>  src/spice-channel.c | 32 +---
>  1 file changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index ff85715..dd32818 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -2124,9 +2124,7 @@ static void spice_channel_flushed(SpiceChannel 
> *channel, gboolean success)
>  GSList *l;
>  
>  for (l = c->flushing; l != NULL; l = l->next) {
> -GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(l->data);
> -g_simple_async_result_set_op_res_gboolean(result, success);
> -g_simple_async_result_complete_in_idle(result);
> +g_task_return_boolean(G_TASK(l->data), success);
>  }
>  
>  g_slist_free_full(c->flushing, g_object_unref);
> @@ -2970,7 +2968,7 @@ static void 
> spice_channel_send_migration_handshake(SpiceChannel *channel)
>  void spice_channel_flush_async(SpiceChannel *self, GCancellable *cancellable,
> GAsyncReadyCallback callback, gpointer 
> user_data)
>  {
> -GSimpleAsyncResult *simple;
> +GTask *task;
>  SpiceChannelPrivate *c;
>  gboolean was_empty;
>  
> @@ -2978,26 +2976,25 @@ void spice_channel_flush_async(SpiceChannel *self, 
> GCancellable *cancellable,
>  c = self->priv;
>  
>  if (c->state != SPICE_CHANNEL_STATE_READY) {
> -g_simple_async_report_error_in_idle(G_OBJECT(self), callback, 
> user_data,
> +g_task_report_new_error(self, callback, user_data,
> +spice_channel_flush_async,
>  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>  "The channel is not ready yet");
>  return;
>  }
>  
> -simple = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
> -   spice_channel_flush_async);
> +task = g_task_new(self, cancellable, callback, user_data);
>  
>  STATIC_MUTEX_LOCK(c->xmit_queue_lock);
>  was_empty = g_queue_is_empty(&c->xmit_queue);
>  STATIC_MUTEX_UNLOCK(c->xmit_queue_lock);
>  if (was_empty) {
> -g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> -g_simple_async_result_complete_in_idle(simple);
> -g_object_unref(simple);
> +g_task_return_boolean(task, TRUE);
> +g_object_unref(task);
>  return;
>  }
>  
> -c->flushing = g_slist_append(c->flushing, simple);
> +c->flushing = g_slist_append(c->flushing, task);
>  }
>  
>  /**
> @@ -3015,19 +3012,16 @@ void spice_channel_flush_async(SpiceChannel *self, 
> GCancellable *cancellable,
>  gboolean spice_channel_flush_finish(SpiceChannel *self, GAsyncResult *result,
>  GError **error)
>  {
> -GSimpleAsyncResult *simple;
> +GTask *task;
>  
>  g_return_val_if_fail(SPICE_IS_CHANNEL(self), FALSE);
>  g_return_val_if_fail(result != NULL, FALSE);
>  
> -simple = (GSimpleAsyncResult *)result;
> +task = G_TASK(result);
>  
> -if (g_simple_async_result_propagate_error(simple, error))
> -return -1;
> -
> -g_return_val_if_fail(g_simple_async_result_is_valid(result, 
> G_OBJECT(self),
> -
> spice_channel_flush_async), FALSE);
> +g_return_val_if_fail(g_task_is_valid(task, self),
> + FALSE);
>  
>  CHANNEL_DEBUG(self, "flushed finished!");
> -return g_simple_async_result_get_op_res_gboolean(simple);
> +return g_task_propagate_boolean(task, error);
>  }
> -- 
> 2.5.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC: drop spice-gtk gtk+ 2.0 support

2016-01-18 Thread Marc-André Lureau

Hi

- Original Message -
> On Mon, Dec 21, 2015 at 4:54 PM, Marc-André Lureau
>  wrote:
> > Hi,
> >
> > With virt-viewer now dropping gtk 2.0, I don't know anyone left using
> > the spice-gtk with gtk+ 2.0. Thus I'd like to propose that the next
> > spice-gtk release be the last with 2.0 support.

fwiw, I also have a branch ready for a while. I'd like to see first local virgl 
support with gtk+2.0 released. Then switching to gtk3/gtkglarea to support 
wayland will be easier. 

cheers
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC: drop spice-gtk gtk+ 2.0 support

2016-01-18 Thread Daniel P. Berrange

On Mon, Jan 18, 2016 at 02:07:49PM +0100, Fabiano Fidêncio wrote:
> On Mon, Dec 21, 2015 at 4:54 PM, Marc-André Lureau
>  wrote:
> > Hi,
> >
> > With virt-viewer now dropping gtk 2.0, I don't know anyone left using
> > the spice-gtk with gtk+ 2.0. Thus I'd like to propose that the next
> > spice-gtk release be the last with 2.0 support.
> >
> > Comments?
> 
> I completely agree with this.
> I already have been working on a patch dropping the support and I will
> submit it as soon we do the last release with gtk2 support.

virt-manager is already gtk3 only too, as is GNOME boxes, so I doubt
that there's any mainstream user of spice-gtk with gtk2, and if there
was they can stick on an older maint branch if really needed

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] RFC: drop spice-gtk gtk+ 2.0 support

2016-01-18 Thread Fabiano Fidêncio

On Mon, Dec 21, 2015 at 4:54 PM, Marc-André Lureau
 wrote:
> Hi,
>
> With virt-viewer now dropping gtk 2.0, I don't know anyone left using
> the spice-gtk with gtk+ 2.0. Thus I'd like to propose that the next
> spice-gtk release be the last with 2.0 support.
>
> Comments?

I completely agree with this.
I already have been working on a patch dropping the support and I will
submit it as soon we do the last release with gtk2 support.

Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] Change some functions to take RedsState arg

2016-01-18 Thread Pavel Grunt

On Mon, 2016-01-18 at 06:07 -0500, Frediano Ziglio wrote:
> > 
> > From: Jonathon Jongsma 
> > 
> > In preparation for getting rid of the global 'reds' variable, we
> > need to
> > pass the RedsState variable to all functions where it is needed.
> > For now
> > the callers just pass in the global reds variable.
> > 
> > Functions changed:
> > - reds_register_channel;
> > - reds_unregister_channel;
> > - reds_get_mouse_mode;
> > - reds_set_mouse_mode;
> > - reds_update_mouse_mode;
> > - reds_agent_remove;
> > - reds_find_channel;
> > - reds_mig_cleanup;
> > - reds_reset_vdp;
> > - reds_main_channel_connected;
> > - reds_client_disconnect;
> > - reds_disconnect;
> > - reds_mig_disconnect.
> > 
> > Acked-by: Frediano Ziglio 
> 
> Does it sound reasonable as comment and patch (just a squash of
> posted ones)?

Yes, it does. It is a simple change

Acked-by: Pavel Grunt 

Pavel

> Should I number the title?
> 
> Frediano
> 
> > ---
> >  server/inputs-channel.c  |  10 ++---
> >  server/main-dispatcher.c |   2 +-
> >  server/red-dispatcher.c  |   4 +-
> >  server/reds.c| 104
> >  +++
> >  server/reds.h|   8 ++--
> >  server/smartcard.c   |   2 +-
> >  server/sound.c   |   6 +--
> >  server/spicevmc.c|   4 +-
> >  8 files changed, 70 insertions(+), 70 deletions(-)
> > 
> > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > index c21cab8..bb5b203 100644
> > --- a/server/inputs-channel.c
> > +++ b/server/inputs-channel.c
> > @@ -364,7 +364,7 @@ static int
> > inputs_channel_handle_parsed(RedChannelClient
> > *rcc, uint32_t size, ui
> >  red_channel_client_pipe_add_type(rcc,
> >  PIPE_ITEM_MOUSE_MOTION_ACK);
> >  icc->motion_count = 0;
> >  }
> > -if (mouse && reds_get_mouse_mode() ==
> > SPICE_MOUSE_MODE_SERVER) {
> > +if (mouse && reds_get_mouse_mode(reds) ==
> > SPICE_MOUSE_MODE_SERVER) {
> >  SpiceMouseInterface *sif;
> >  sif = SPICE_CONTAINEROF(mouse->base.sif,
> > SpiceMouseInterface,
> >  base);
> >  sif->motion(mouse,
> > @@ -381,7 +381,7 @@ static int
> > inputs_channel_handle_parsed(RedChannelClient
> > *rcc, uint32_t size, ui
> >  red_channel_client_pipe_add_type(rcc,
> >  PIPE_ITEM_MOUSE_MOTION_ACK);
> >  icc->motion_count = 0;
> >  }
> > -if (reds_get_mouse_mode() != SPICE_MOUSE_MODE_CLIENT) {
> > +if (reds_get_mouse_mode(reds) != SPICE_MOUSE_MODE_CLIENT)
> > {
> >  break;
> >  }
> >  spice_assert((reds_get_agent_mouse() &&
> > reds_has_vdagent()) ||
> >  tablet);
> > @@ -407,7 +407,7 @@ static int
> > inputs_channel_handle_parsed(RedChannelClient
> > *rcc, uint32_t size, ui
> >  } else if (mouse_press->button == SPICE_MOUSE_BUTTON_DOWN)
> > {
> >  dz = 1;
> >  }
> > -if (reds_get_mouse_mode() == SPICE_MOUSE_MODE_CLIENT) {
> > +if (reds_get_mouse_mode(reds) == SPICE_MOUSE_MODE_CLIENT)
> > {
> >  if (reds_get_agent_mouse() && reds_has_vdagent()) {
> >  inputs_channel->mouse_state.buttons =
> >  RED_MOUSE_BUTTON_STATE_TO_AGENT(mouse_press-
> > >buttons_state)
> >  |
> > @@ -429,7 +429,7 @@ static int
> > inputs_channel_handle_parsed(RedChannelClient
> > *rcc, uint32_t size, ui
> >  }
> >  case SPICE_MSGC_INPUTS_MOUSE_RELEASE: {
> >  SpiceMsgcMouseRelease *mouse_release = message;
> > -if (reds_get_mouse_mode() == SPICE_MOUSE_MODE_CLIENT) {
> > +if (reds_get_mouse_mode(reds) == SPICE_MOUSE_MODE_CLIENT)
> > {
> >  if (reds_get_agent_mouse() && reds_has_vdagent()) {
> >  inputs_channel->mouse_state.buttons =
> >  RED_MOUSE_BUTTON_STATE_TO_AGENT(mouse_release-
> > >buttons_state);
> > @@ -672,7 +672,7 @@ void inputs_init(void)
> >  red_channel_register_client_cbs(&g_inputs_channel->base,
> > &client_cbs);
> >  
> >  red_channel_set_cap(&g_inputs_channel->base,
> >  SPICE_INPUTS_CAP_KEY_SCANCODE);
> > -reds_register_channel(&g_inputs_channel->base);
> > +reds_register_channel(reds, &g_inputs_channel->base);
> >  
> >  if (!(key_modifiers_timer = core->timer_add(core,
> > key_modifiers_sender,
> >  NULL))) {
> >  spice_error("key modifiers timer create failed");
> > diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
> > index 2cb53ef..db87e05 100644
> > --- a/server/main-dispatcher.c
> > +++ b/server/main-dispatcher.c
> > @@ -136,7 +136,7 @@ static void
> > main_dispatcher_handle_client_disconnect(void
> > *opaque,
> >  MainDispatcherClientDisconnectMessage *msg = payload;
> >  
> >  spice_debug("client=%p", msg->client);
> > -reds_client_disconnect(msg->client);
> > +reds_client_disconnect(reds, msg->client);
> >  red_client_unref(msg->client);
> >  }

Re: [Spice-devel] [PATCH spice v2 3/4] dcc: make dcc_compress_image_*() private

Hi,

Looks good
Reviewed-by: Victor Toso 

On Fri, Jan 15, 2016 at 05:11:34PM +0100, Pavel Grunt wrote:
> ---
>  server/dcc.c | 30 +++---
>  server/dcc.h | 15 ---
>  2 files changed, 15 insertions(+), 30 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index 097140a..9b65031 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -646,9 +646,9 @@ static const LzImageType bitmap_fmt_to_lz_image_type[] = {
>  
>  #define MIN_GLZ_SIZE_FOR_ZLIB 100
>  
> -int dcc_compress_image_glz(DisplayChannelClient *dcc,
> -   SpiceImage *dest, SpiceBitmap *src, Drawable 
> *drawable,
> -   compress_send_data_t* o_comp_data)
> +static int dcc_compress_image_glz(DisplayChannelClient *dcc,
> +  SpiceImage *dest, SpiceBitmap *src, 
> Drawable *drawable,
> +  compress_send_data_t* o_comp_data)
>  {
>  DisplayChannel *display_channel = DCC_TO_DC(dcc);
>  stat_start_time_t start_time;
> @@ -723,9 +723,9 @@ glz:
>  return TRUE;
>  }
>  
> -int dcc_compress_image_lz(DisplayChannelClient *dcc,
> -  SpiceImage *dest, SpiceBitmap *src,
> -  compress_send_data_t* o_comp_data, uint32_t 
> group_id)
> +static int dcc_compress_image_lz(DisplayChannelClient *dcc,
> + SpiceImage *dest, SpiceBitmap *src,
> + compress_send_data_t* o_comp_data, uint32_t 
> group_id)
>  {
>  LzData *lz_data = &dcc->lz_data;
>  LzContext *lz = dcc->lz;
> @@ -785,9 +785,9 @@ int dcc_compress_image_lz(DisplayChannelClient *dcc,
>  return TRUE;
>  }
>  
> -int dcc_compress_image_jpeg(DisplayChannelClient *dcc, SpiceImage *dest,
> -SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> -uint32_t group_id)
> +static int dcc_compress_image_jpeg(DisplayChannelClient *dcc, SpiceImage 
> *dest,
> +   SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> +   uint32_t group_id)
>  {
>  JpegData *jpeg_data = &dcc->jpeg_data;
>  LzData *lz_data = &dcc->lz_data;
> @@ -910,9 +910,9 @@ int dcc_compress_image_jpeg(DisplayChannelClient *dcc, 
> SpiceImage *dest,
>  }
>  
>  #ifdef USE_LZ4
> -int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage *dest,
> -   SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> -   uint32_t group_id)
> +static int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage 
> *dest,
> +  SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> +  uint32_t group_id)
>  {
>  Lz4Data *lz4_data = &dcc->lz4_data;
>  Lz4EncoderContext *lz4 = dcc->lz4;
> @@ -957,9 +957,9 @@ int dcc_compress_image_lz4(DisplayChannelClient *dcc, 
> SpiceImage *dest,
>  }
>  #endif
>  
> -int dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage *dest,
> -SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> -uint32_t group_id)
> +static int dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage 
> *dest,
> +   SpiceBitmap *src, compress_send_data_t* 
> o_comp_data,
> +   uint32_t group_id)
>  {
>  QuicData *quic_data = &dcc->quic_data;
>  QuicContext *quic = dcc->quic;
> diff --git a/server/dcc.h b/server/dcc.h
> index 2a12226..f715792 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -219,20 +219,5 @@ intdcc_compress_image
> (DisplayCha
>
> SpiceImage *dest, SpiceBitmap *src, Drawable *drawable,
>int 
> can_lossy,
>
> compress_send_data_t* o_comp_data);
> -intdcc_compress_image_glz
> (DisplayChannelClient *dcc,
> -  
> SpiceImage *dest, SpiceBitmap *src, Drawable *drawable,
> -  
> compress_send_data_t* o_comp_data);
> -intdcc_compress_image_lz 
> (DisplayChannelClient *dcc,
> -  
> SpiceImage *dest, SpiceBitmap *src,
> -  
> compress_send_data_t* o_comp_data, uint32_t group_id);
> -intdcc_compress_image_jpeg   
> (DisplayChannelClient *dcc, SpiceImage *dest,
> -

Re: [Spice-devel] [PATCH spice v2 2/4] dcc-send: Use dcc_compress_image to compress image

Hi,

On Fri, Jan 15, 2016 at 05:11:33PM +0100, Pavel Grunt wrote:
> ---
>  server/dcc-send.c | 44 +++-
>  1 file changed, 3 insertions(+), 41 deletions(-)
> 
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index c3f79ef..58e654e 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -1920,20 +1920,14 @@ static void red_marshall_image(RedChannelClient *rcc, 
> SpiceMarshaller *m, ImageI
>  DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>  DisplayChannel *display = DCC_TO_DC(dcc);
>  SpiceImage red_image;
> -RedWorker *worker;
>  SpiceBitmap bitmap;
>  SpiceChunks *chunks;
>  QRegion *surface_lossy_region;
> -int comp_succeeded = FALSE;
> -int lossy_comp = FALSE;
> -int quic_comp = FALSE;
> -SpiceImageCompression comp_mode;
>  SpiceMsgDisplayDrawCopy copy;
>  SpiceMarshaller *src_bitmap_out, *mask_bitmap_out;
>  SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out;
>  
>  spice_assert(rcc && display && item);
> -worker = display->common.worker;
>  
>  QXL_SET_IMAGE_ID(&red_image, QXL_IMAGE_GROUP_RED, 
> display_channel_generate_uid(display));
>  red_image.descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
> @@ -1981,44 +1975,12 @@ static void red_marshall_image(RedChannelClient *rcc, 
> SpiceMarshaller *m, ImageI
>  
>  compress_send_data_t comp_send_data = {0};
>  
> -comp_mode = dcc->image_compression;
> -
> -if (((comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> -(comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) && 
> !bitmap_has_extra_stride(&bitmap)) {
> -
> -if (bitmap_fmt_has_graduality(item->image_format)) {
> -BitmapGradualType grad_level;
> -
> -grad_level = bitmap_get_graduality_level(&bitmap);
> -if (grad_level == BITMAP_GRADUAL_HIGH) {
> -// if we use lz for alpha, the stride can't be extra
> -lossy_comp = display->enable_jpeg && item->can_lossy;
> -quic_comp = TRUE;
> -}
> -}
> -} else if (comp_mode == SPICE_IMAGE_COMPRESSION_QUIC) {
> -quic_comp = TRUE;
> -}
> -
> -uint32_t groupid = red_worker_get_memslot(worker)->internal_groupslot_id;
> -
> -if (lossy_comp) {
> -comp_succeeded = dcc_compress_image_jpeg(dcc, &red_image, &bitmap, 
> &comp_send_data, groupid);
> -} else if (quic_comp) {
> -comp_succeeded = dcc_compress_image_quic(dcc, &red_image, &bitmap, 
> &comp_send_data, groupid);
> -#ifdef USE_LZ4
> -} else if (comp_mode == SPICE_IMAGE_COMPRESSION_LZ4 &&
> -   bitmap_fmt_is_rgb(bitmap.format) &&
> -   red_channel_client_test_remote_cap(&dcc->common.base,
> -  
> SPICE_DISPLAY_CAP_LZ4_COMPRESSION)) {
> -comp_succeeded = dcc_compress_image_lz4(dcc, &red_image, &bitmap, 
> &comp_send_data, groupid);
> -#endif
> -} else if (comp_mode != SPICE_IMAGE_COMPRESSION_OFF) {
> -comp_succeeded = dcc_compress_image_lz(dcc, &red_image, &bitmap, 
> &comp_send_data, groupid);
> -}
> +int comp_succeeded = dcc_compress_image(dcc, &red_image, &bitmap, NULL, 
> item->can_lossy, &comp_send_data);
>
>  surface_lossy_region = 
> &dcc->surface_client_lossy_region[item->surface_id];
>  if (comp_succeeded) {
> +int lossy_comp = red_image.descriptor.type == SPICE_IMAGE_TYPE_JPEG 
> ||
> + red_image.descriptor.type == 
> SPICE_IMAGE_TYPE_JPEG_ALPHA;

This is not explained.. so, dcc_compress_image takes it->can_lossy but
you need to verify it in the end. Maybe a helper function would be more
clear?

>  spice_marshall_Image(src_bitmap_out, &red_image,
>   &bitmap_palette_out, &lzplt_palette_out);
>  
> -- 
> 2.5.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice v2 1/4] dcc_compress_image: Handle NULL drawable