[Spice-devel] [PATCH] streaming: Tweak the GStreamer decoder to avoid a compiler warning

2016-05-30 Thread Francois Gouget 
We check that there is a matching frame in the queue before popping the
old ones. So we know the inner loop will find a match and thus that
frame will not be NULL. But figuring that out is too hard for the
compiler.

Signed-off-by: Francois Gouget 
---

Adding an if (frame) check suggests that frame can indeed be NULL which 
is not the case (verifying that we will find a match is the point of the 
outer loop). So if this patch gets the compiler off our back it may be a 
better solution.

 src/channel-display-gst.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 46a85ea..87741d9 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -174,7 +174,11 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink, 
gpointer video_decoder)
 while (l) {
 frame = l->data;
 if (frame->timestamp == GST_BUFFER_PTS(buffer)) {
-/* Now that we know there is a match, remove the older
+/* The frame is now ready for display */
+frame->sample = sample;
+g_queue_push_tail(decoder->display_queue, frame);
+
+/* Now that we know there is a match, remove it and the older
  * frames from the decoding queue.
  */
 while ((frame = g_queue_pop_head(decoder->decoding_queue))) {
@@ -187,10 +191,6 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink, 
gpointer video_decoder)
 SPICE_DEBUG("the GStreamer pipeline dropped a frame");
 free_frame(frame);
 }
-
-/* The frame is now ready for display */
-frame->sample = sample;
-g_queue_push_tail(decoder->display_queue, frame);
 break;
 }
 l = l->next;
-- 
2.8.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk 4/7] gst: fix potential null pointer dereference

2016-05-30 Thread Marc-André Lureau 
CC'ing Francois

- Original Message -
> CC   channel-display-gst.lo
> channel-display-gst.c: In function ‘new_sample’:
> channel-display-gst.c:192:31: error: potential null pointer dereference
> [-Werror=null-dereference]
>  frame->sample = sample;
>  ~~^~~~
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  src/channel-display-gst.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 46a85ea..f162078 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -189,8 +189,11 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink,
> gpointer video_decoder)
>  }
>  
>  /* The frame is now ready for display */
> -frame->sample = sample;
> -g_queue_push_tail(decoder->display_queue, frame);
> +if (frame) {
> +frame->sample = sample;
> +g_queue_push_tail(decoder->display_queue, frame);
> +}
> +
>  break;
>  }
>  l = l->next;
> --
> 2.7.4
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-gtk 4/7] gst: fix potential null pointer dereference

2016-05-30 Thread Marc-André Lureau 
CC   channel-display-gst.lo
channel-display-gst.c: In function ‘new_sample’:
channel-display-gst.c:192:31: error: potential null pointer dereference 
[-Werror=null-dereference]
 frame->sample = sample;
 ~~^~~~

Signed-off-by: Marc-André Lureau 
---
 src/channel-display-gst.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 46a85ea..f162078 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -189,8 +189,11 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink, 
gpointer video_decoder)
 }
 
 /* The frame is now ready for display */
-frame->sample = sample;
-g_queue_push_tail(decoder->display_queue, frame);
+if (frame) {
+frame->sample = sample;
+g_queue_push_tail(decoder->display_queue, frame);
+}
+
 break;
 }
 l = l->next;
-- 
2.7.4

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

[Spice-devel] [PATCH spice-gtk 7/7] build-sys: update manywarnings.m4

2016-05-30 Thread Marc-André Lureau 
Signed-off-by: Marc-André Lureau 
---
 m4/manywarnings.m4 | 39 ++-
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 3e6dd21..90823b0 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -1,5 +1,5 @@
-# manywarnings.m4 serial 7
-dnl Copyright (C) 2008-2014 Free Software Foundation, Inc.
+# manywarnings.m4 serial 8
+dnl Copyright (C) 2008-2016 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
@@ -108,12 +108,13 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Waddress \
 -Waggressive-loop-optimizations \
 -Wall \
--Warray-bounds \
 -Wattributes \
 -Wbad-function-cast \
+-Wbool-compare \
 -Wbuiltin-macro-redefined \
 -Wcast-align \
 -Wchar-subscripts \
+-Wchkp \
 -Wclobbered \
 -Wcomment \
 -Wcomments \
@@ -122,9 +123,13 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Wdate-time \
 -Wdeprecated \
 -Wdeprecated-declarations \
+-Wdesignated-init \
 -Wdisabled-optimization \
+-Wdiscarded-array-qualifiers \
+-Wdiscarded-qualifiers \
 -Wdiv-by-zero \
 -Wdouble-promotion \
+-Wduplicated-cond \
 -Wempty-body \
 -Wendif-labels \
 -Wenum-compare \
@@ -133,22 +138,31 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Wformat-extra-args \
 -Wformat-nonliteral \
 -Wformat-security \
+-Wformat-signedness \
 -Wformat-y2k \
 -Wformat-zero-length \
+-Wframe-address \
 -Wfree-nonheap-object \
+-Whsa \
+-Wignored-attributes \
 -Wignored-qualifiers \
 -Wimplicit \
 -Wimplicit-function-declaration \
 -Wimplicit-int \
+-Wincompatible-pointer-types \
 -Winit-self \
 -Winline \
+-Wint-conversion \
 -Wint-to-pointer-cast \
 -Winvalid-memory-model \
 -Winvalid-pch \
 -Wjump-misses-init \
+-Wlogical-not-parentheses \
 -Wlogical-op \
 -Wmain \
 -Wmaybe-uninitialized \
+-Wmemset-transposed-args \
+-Wmisleading-indentation \
 -Wmissing-braces \
 -Wmissing-declarations \
 -Wmissing-field-initializers \
@@ -159,6 +173,9 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Wnarrowing \
 -Wnested-externs \
 -Wnonnull \
+-Wnonnull-compare \
+-Wnull-dereference \
+-Wodr \
 -Wold-style-declaration \
 -Wold-style-definition \
 -Wopenmp-simd \
@@ -174,8 +191,13 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Wpragmas \
 -Wreturn-local-addr \
 -Wreturn-type \
+-Wscalar-storage-order \
 -Wsequence-point \
 -Wshadow \
+-Wshift-count-negative \
+-Wshift-count-overflow \
+-Wshift-negative-value \
+-Wsizeof-array-argument \
 -Wsizeof-pointer-memaccess \
 -Wstack-protector \
 -Wstrict-aliasing \
@@ -185,10 +207,14 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 -Wsuggest-attribute=format \
 -Wsuggest-attribute=noreturn \
 -Wsuggest-attribute=pure \
+-Wsuggest-final-methods \
+-Wsuggest-final-types \
 -Wswitch \
+-Wswitch-bool \
 -Wswitch-default \
 -Wsync-nand \
 -Wsystem-headers \
+-Wtautological-compare \
 -Wtrampolines \
 -Wtrigraphs \
 -Wtype-limits \
@@ -217,9 +243,12 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
 gl_manywarn_set="$gl_manywarn_set $gl_manywarn_item"
   done
 
-  # gcc --help=warnings outputs an unusual form for this option; list
-  # it here so that the above 'comm' command doesn't report a false match.
+  # gcc --help=warnings outputs an unusual form for these options; list
+  # them here so that the above 'comm' command doesn't report a false match.
+  gl_manywarn_set="$gl_manywarn_set -Warray-bounds=2"
   gl_manywarn_set="$gl_manywarn_set -Wnormalized=nfc"
+  gl_manywarn_set="$gl_manywarn_set -Wshift-overflow=2"
+  gl_manywarn_set="$gl_manywarn_set -Wunused-const-variable=2"
 
   # These are needed for older GCC versions.
   if test -n "$GCC"; then
-- 
2.7.4

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

[Spice-devel] [PATCH spice-gtk 5/7] Fix many -Werror=format warnings

2016-05-30 Thread Marc-André Lureau 
Signed-off-by: Marc-André Lureau 
---
 src/channel-base.c |  2 +-
 src/channel-cursor.c   |  2 +-
 src/channel-display-gst.c  |  4 +--
 src/channel-display-mjpeg.c|  2 +-
 src/channel-display.c  | 20 +++---
 src/channel-main.c | 20 +++---
 src/channel-playback.c |  4 +--
 src/channel-record.c   |  2 +-
 src/channel-smartcard.c|  8 +++---
 src/decode-glz.c   |  4 +--
 src/spice-channel.c| 50 +-
 src/spice-client-glib-usb-acl-helper.c |  2 +-
 src/spice-gstaudio.c   |  4 +--
 src/spice-gtk-session.c|  6 ++--
 src/spice-pulse.c  |  8 +++---
 src/spice-session.c|  4 +--
 src/spice-util.c   |  2 +-
 src/spice-widget-egl.c |  5 ++--
 src/spice-widget.c | 12 
 src/spicy-screenshot.c |  4 +--
 src/spicy-stats.c  |  2 +-
 src/spicy.c|  2 +-
 src/usb-device-manager.c   |  2 +-
 src/usbutil.c  |  2 +-
 24 files changed, 87 insertions(+), 86 deletions(-)

diff --git a/src/channel-base.c b/src/channel-base.c
index 20bb0cc..004dba9 100644
--- a/src/channel-base.c
+++ b/src/channel-base.c
@@ -77,7 +77,7 @@ spice_channel_handle_notify(SpiceChannel *channel, SpiceMsgIn 
*in)
 
 CHANNEL_DEBUG(channel, "%s -- %s%s #%u%s%.*s", __FUNCTION__,
 severity, visibility, notify->what,
-message_str ? ": " : "", notify->message_len,
+message_str ? ": " : "", (int)notify->message_len,
 message_str ? message_str : "");
 }
 
diff --git a/src/channel-cursor.c b/src/channel-cursor.c
index 2ae8f86..609243b 100644
--- a/src/channel-cursor.c
+++ b/src/channel-cursor.c
@@ -304,7 +304,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, 
SpiceCursor *scursor)
 guint8 *rgba;
 guint8 val;
 
-CHANNEL_DEBUG(channel, "%s: flags %d, size %d", __FUNCTION__,
+CHANNEL_DEBUG(channel, "%s: flags %x, size %u", __FUNCTION__,
   scursor->flags, scursor->data_size);
 
 if (scursor->flags & SPICE_CURSOR_FLAGS_NONE)
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index f162078..7a33c49 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -369,7 +369,7 @@ static void spice_gst_decoder_queue_frame(VideoDecoder 
*video_decoder,
 SpiceStreamDataHeader *frame_op = spice_msg_in_parsed(frame_msg);
 if (frame_op->multi_media_time < decoder->last_mm_time) {
 SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):"
-" resetting stream, id %d",
+" resetting stream, id %u",
 frame_op->multi_media_time,
 decoder->last_mm_time, frame_op->id);
 /* Let GStreamer deal with the frame anyway */
@@ -405,7 +405,7 @@ static void spice_gst_decoder_queue_frame(VideoDecoder 
*video_decoder,
 g_mutex_unlock(>queues_mutex);
 
 if (gst_app_src_push_buffer(decoder->appsrc, buffer) != GST_FLOW_OK) {
-SPICE_DEBUG("GStreamer error: unable to push frame of size %d", size);
+SPICE_DEBUG("GStreamer error: unable to push frame of size %u", size);
 stream_dropped_frame_on_playback(decoder->base.stream);
 }
 }
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 1238b41..a2dae82 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -250,7 +250,7 @@ static void mjpeg_decoder_queue_frame(VideoDecoder 
*video_decoder,
 if (frame_op->multi_media_time < last_op->multi_media_time) {
 /* This should really not happen */
 SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):"
-" resetting stream, id %d",
+" resetting stream, id %u",
 frame_op->multi_media_time,
 last_op->multi_media_time, frame_op->id);
 mjpeg_decoder_drop_queue(decoder);
diff --git a/src/channel-display.c b/src/channel-display.c
index a1ed493..54bc30e 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1086,7 +1086,7 @@ static void display_handle_stream_create(SpiceChannel 
*channel, SpiceMsgIn *in)
 SpiceMsgDisplayStreamCreate *op = spice_msg_in_parsed(in);
 display_stream *st;
 
-CHANNEL_DEBUG(channel, "%s: id %d", __FUNCTION__, op->id);
+CHANNEL_DEBUG(channel, "%s: id %u", __FUNCTION__, op->id);
 
 if (op->id >= c->nstreams) {
 int n = c->nstreams;
@@ -1127,7 +1127,7 @@ static void display_handle_stream_create(SpiceChannel 
*channel, SpiceMsgIn *in)
 #endif
 }
 if (st->video_decoder == NULL) {
-spice_printerr("could not create a

[Spice-devel] [PATCH spice-gtk 6/7] build-sys: -Wshift-overflow=2

2016-05-30 Thread Marc-André Lureau 
manywarnings.m4 update will bring new flags that fail with some
glib/gst headers.

Signed-off-by: Marc-André Lureau 
---
 configure.ac | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 8f1e91b..3fe8055 100644
--- a/configure.ac
+++ b/configure.ac
@@ -581,7 +581,10 @@ SPICE_CHECK_LZ4
 dnl ===
 dnl check compiler flags
 
-SPICE_COMPILE_WARNINGS
+# some glib/gstreamer enums use 1 << 31
+dontwarn="-Wshift-overflow=2"
+
+SPICE_COMPILE_WARNINGS([$dontwarn])
 
 SPICE_CFLAGS="$SPICE_CFLAGS $WARN_CFLAGS"
 
-- 
2.7.4

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

[Spice-devel] [PATCH spice-gtk 3/7] build-sys: remove -Wmissing-declarations

2016-05-30 Thread Marc-André Lureau 
The warning doesn't show up anymore.

Signed-off-by: Marc-André Lureau 
---
 configure.ac | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3c90e9e..8f1e91b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -581,11 +581,7 @@ SPICE_CHECK_LZ4
 dnl ===
 dnl check compiler flags
 
-# We want to enable these, but need to sort out the
-# decl mess with  src/generated_*.c
-dontwarn="-Wmissing-declarations"
-
-SPICE_COMPILE_WARNINGS([$dontwarn])
+SPICE_COMPILE_WARNINGS
 
 SPICE_CFLAGS="$SPICE_CFLAGS $WARN_CFLAGS"
 
-- 
2.7.4

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

[Spice-devel] [PATCH spice-gtk 2/7] build-sys: remove some gtk+ 2.0 warnings flags

2016-05-30 Thread Marc-André Lureau 
As we dropped gtk+ 2.0 anyway.

Signed-off-by: Marc-André Lureau 
---
 configure.ac | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index c94d41b..3c90e9e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -585,13 +585,6 @@ dnl check compiler flags
 # decl mess with  src/generated_*.c
 dontwarn="-Wmissing-declarations"
 
-# We want to enable these, but Gtk+2.0 has a bad decl
-# gtk-2.0/gtk/gtkitemfactory.h:47:1: error: function declaration
-# isn't a prototype.
-if test "$with_gtk" = "2.0"; then
-  dontwarn="$dontwarn -Wstrict-prototypes"
-fi
-
 SPICE_COMPILE_WARNINGS([$dontwarn])
 
 SPICE_CFLAGS="$SPICE_CFLAGS $WARN_CFLAGS"
-- 
2.7.4

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

[Spice-devel] [PATCH spice-gtk 1/7] build-sys: enable -Wmissing-prototypes

2016-05-30 Thread Marc-André Lureau 
Turns out it is possible to fix the warnings now.

Signed-off-by: Marc-André Lureau 
---
 configure.ac   |  2 +-
 src/channel-base.c | 20 ++--
 src/channel-playback.c |  1 +
 src/giopipe.c  |  6 +-
 src/spice-gstaudio.c   |  2 +-
 src/spice-uri.c|  4 ++--
 src/spicy.c| 13 +
 7 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7f29c66..c94d41b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -583,7 +583,7 @@ dnl check compiler flags
 
 # We want to enable these, but need to sort out the
 # decl mess with  src/generated_*.c
-dontwarn="-Wmissing-prototypes -Wmissing-declarations"
+dontwarn="-Wmissing-declarations"
 
 # We want to enable these, but Gtk+2.0 has a bad decl
 # gtk-2.0/gtk/gtkitemfactory.h:47:1: error: function declaration
diff --git a/src/channel-base.c b/src/channel-base.c
index de04b89..20bb0cc 100644
--- a/src/channel-base.c
+++ b/src/channel-base.c
@@ -24,8 +24,8 @@
 #include "spice-channel-priv.h"
 
 /* coroutine context */
-G_GNUC_INTERNAL
-void spice_channel_handle_set_ack(SpiceChannel *channel, SpiceMsgIn *in)
+static void
+spice_channel_handle_set_ack(SpiceChannel *channel, SpiceMsgIn *in)
 {
 SpiceChannelPrivate *c = channel->priv;
 SpiceMsgSetAck* ack = spice_msg_in_parsed(in);
@@ -40,8 +40,8 @@ void spice_channel_handle_set_ack(SpiceChannel *channel, 
SpiceMsgIn *in)
 }
 
 /* coroutine context */
-G_GNUC_INTERNAL
-void spice_channel_handle_ping(SpiceChannel *channel, SpiceMsgIn *in)
+static void
+spice_channel_handle_ping(SpiceChannel *channel, SpiceMsgIn *in)
 {
 SpiceChannelPrivate *c = channel->priv;
 SpiceMsgPing *ping = spice_msg_in_parsed(in);
@@ -52,8 +52,8 @@ void spice_channel_handle_ping(SpiceChannel *channel, 
SpiceMsgIn *in)
 }
 
 /* coroutine context */
-G_GNUC_INTERNAL
-void spice_channel_handle_notify(SpiceChannel *channel, SpiceMsgIn *in)
+static void
+spice_channel_handle_notify(SpiceChannel *channel, SpiceMsgIn *in)
 {
 static const char* severity_strings[] = {"info", "warn", "error"};
 static const char* visibility_strings[] = {"!", "!!", "!!!"};
@@ -82,8 +82,8 @@ void spice_channel_handle_notify(SpiceChannel *channel, 
SpiceMsgIn *in)
 }
 
 /* coroutine context */
-G_GNUC_INTERNAL
-void spice_channel_handle_disconnect(SpiceChannel *channel, SpiceMsgIn *in)
+static void
+spice_channel_handle_disconnect(SpiceChannel *channel, SpiceMsgIn *in)
 {
 SpiceMsgDisconnect *disconnect = spice_msg_in_parsed(in);
 
@@ -148,8 +148,8 @@ get_msg_handler(SpiceChannel *channel, SpiceMsgIn *in, 
gpointer data)
 }
 
 /* coroutine context */
-G_GNUC_INTERNAL
-void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
+static void
+spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
 {
 SpiceMsgOut *out;
 SpiceMsgIn *data = NULL;
diff --git a/src/channel-playback.c b/src/channel-playback.c
index e931727..8edcb22 100644
--- a/src/channel-playback.c
+++ b/src/channel-playback.c
@@ -25,6 +25,7 @@
 #include "spice-marshal.h"
 
 #include "common/snd_codec.h"
+#include "channel-playback-priv.h"
 
 /**
  * SECTION:channel-playback
diff --git a/src/giopipe.c b/src/giopipe.c
index d91c4d9..e76090c 100644
--- a/src/giopipe.c
+++ b/src/giopipe.c
@@ -81,6 +81,8 @@ static void pipe_input_stream_pollable_iface_init 
(GPollableInputStreamInterface
 static void pipe_input_stream_check_source (PipeInputStream *self);
 static void pipe_output_stream_check_source (PipeOutputStream *self);
 
+static GType pipe_input_stream_get_type(void);
+
 G_DEFINE_TYPE_WITH_CODE (PipeInputStream, pipe_input_stream, 
G_TYPE_INPUT_STREAM,
  G_IMPLEMENT_INTERFACE (G_TYPE_POLLABLE_INPUT_STREAM,
 
pipe_input_stream_pollable_iface_init))
@@ -267,6 +269,8 @@ pipe_input_stream_pollable_iface_init 
(GPollableInputStreamInterface *iface)
 
 static void pipe_output_stream_pollable_iface_init 
(GPollableOutputStreamInterface *iface);
 
+static GType pipe_output_stream_get_type(void);
+
 G_DEFINE_TYPE_WITH_CODE (PipeOutputStream, pipe_output_stream, 
G_TYPE_OUTPUT_STREAM,
  G_IMPLEMENT_INTERFACE (G_TYPE_POLLABLE_OUTPUT_STREAM,
 
pipe_output_stream_pollable_iface_init))
@@ -438,7 +442,7 @@ pipe_output_stream_pollable_iface_init 
(GPollableOutputStreamInterface *iface)
 iface->create_source = pipe_output_stream_create_source;
 }
 
-G_GNUC_INTERNAL void
+static void
 make_gio_pipe(GInputStream **input, GOutputStream **output)
 {
 PipeInputStream *in;
diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
index 9a92452..12e6d66 100644
--- a/src/spice-gstaudio.c
+++ b/src/spice-gstaudio.c
@@ -66,7 +66,7 @@ static void spice_gstaudio_finalize(GObject *obj)
 G_OBJECT_CLASS(spice_gstaudio_parent_class)->finalize(obj);
 }
 
-void stream_dispose(struct stream *s)
+static void

Re: [Spice-devel] [spice v15 11/21] streaming: Avoid copying the input frame in the GStreamer encoder

2016-05-30 Thread Francois Gouget 
Note that we can only avoid copies for the first 1 Mpixels or so.
That's because Spice splits larger frames into more chunks than we can
fit GstMemory fragments in a GStreamer buffer. So if there are more
pixels we will avoid copies for the first 3840 KB and copy the rest.
Furthermore, while in practice the GStreamer encoder will only modify
the RedDrawable refcount during the encode_frame(), in theory the
refcount could be decremented from the GStreamer thread after
encode_frame() returns.

Signed-off-by: Francois Gouget 
---

Adjusted for the flipped is_chunk_stride_aligned() return value.

 server/dcc-send.c  |  14 ++--
 server/gstreamer-encoder.c | 199 +
 server/mjpeg-encoder.c |   5 +-
 server/stream.c|  16 +++-
 server/video-encoder.h |  25 +-
 5 files changed, 231 insertions(+), 28 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 43299c7..a0eaf7f 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -1658,12 +1658,12 @@ static void red_release_video_encoder_buffer(uint8_t 
*data, void *opaque)
 }
 
 static int red_marshall_stream_data(RedChannelClient *rcc,
-SpiceMarshaller *base_marshaller, Drawable 
*drawable)
+SpiceMarshaller *base_marshaller,
+Drawable *drawable)
 {
 DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
 DisplayChannel *display = DCC_TO_DC(dcc);
 Stream *stream = drawable->stream;
-SpiceImage *image;
 uint32_t frame_mm_time;
 int ret;
 
@@ -1671,10 +1671,10 @@ static int red_marshall_stream_data(RedChannelClient 
*rcc,
 spice_assert(drawable->sized_stream);
 stream = drawable->sized_stream;
 }
-spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
-
-image = drawable->red_drawable->u.copy.src_bitmap;
+RedDrawable *red_drawable = drawable->red_drawable;
+spice_assert(red_drawable->type == QXL_DRAW_COPY);
 
+SpiceImage *image = red_drawable->u.copy.src_bitmap;
 if (image->descriptor.type != SPICE_IMAGE_TYPE_BITMAP) {
 return FALSE;
 }
@@ -1706,8 +1706,8 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
 ret = !agent->video_encoder ? VIDEO_ENCODER_FRAME_UNSUPPORTED :
   agent->video_encoder->encode_frame(agent->video_encoder,
  frame_mm_time,
- >u.bitmap,
- src_area, stream->top_down,
+ >u.bitmap, src_area,
+ stream->top_down, red_drawable,
  );
 switch (ret) {
 case VIDEO_ENCODER_FRAME_DROP:
diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index abaf2ae..a698bb1 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -30,6 +30,8 @@
 
 #define SPICE_GST_DEFAULT_FPS 30
 
+#define DO_ZERO_COPY
+
 
 typedef struct {
 SpiceBitmapFmt spice_format;
@@ -46,6 +48,14 @@ typedef struct SpiceGstVideoBuffer {
 typedef struct SpiceGstEncoder {
 VideoEncoder base;
 
+/* Callbacks to adjust the refcount of the bitmap being encoded. */
+bitmap_ref_t bitmap_ref;
+bitmap_unref_t bitmap_unref;
+
+#ifdef DO_ZERO_COPY
+GAsyncQueue *unused_bitmap_opaques;
+#endif
+
 /* Rate control callbacks */
 VideoEncoderRateControlCbs cbs;
 
@@ -486,12 +496,109 @@ static inline int line_copy(SpiceGstEncoder *encoder, 
const SpiceBitmap *bitmap,
  return TRUE;
 }
 
+#ifdef DO_ZERO_COPY
+typedef struct {
+gint refs;
+SpiceGstEncoder *encoder;
+gpointer opaque;
+} BitmapWrapper;
+
+static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean 
unref_queue)
+{
+gpointer bitmap_opaque;
+while ((bitmap_opaque = 
g_async_queue_try_pop(encoder->unused_bitmap_opaques))) {
+encoder->bitmap_unref(bitmap_opaque);
+}
+if (unref_queue) {
+g_async_queue_unref(encoder->unused_bitmap_opaques);
+}
+}
+
+static BitmapWrapper *bitmap_wrapper_new(SpiceGstEncoder *encoder, gpointer 
bitmap_opaque)
+{
+BitmapWrapper *wrapper = spice_new(BitmapWrapper, 1);
+wrapper->refs = 1;
+wrapper->encoder = encoder;
+wrapper->opaque = bitmap_opaque;
+encoder->bitmap_ref(bitmap_opaque);
+return wrapper;
+}
+
+static void bitmap_wrapper_unref(gpointer data)
+{
+BitmapWrapper *wrapper = data;
+if (g_atomic_int_dec_and_test(>refs)) {
+g_async_queue_push(wrapper->encoder->unused_bitmap_opaques, 
wrapper->opaque);
+free(wrapper);
+}
+}
+
+
+/* A helper for push_raw_frame() */
+static inline int zero_copy(SpiceGstEncoder *encoder,
+const SpiceBitmap *bitmap, gpointer bitmap_opaque,
+GstBuffer *buffer, uint32_t *chunk_index,
+

Re: [Spice-devel] [spice v15 03/21] streaming: Add a GStreamer 1.0 MJPEG video encoder and use it by default

2016-05-30 Thread Francois Gouget 
This introduces a pared down GStreamer-based video encoder to serve as
the basis for later enhancements.
In this form the new encoder supports both regular and sized streams
but lacks any rate control. It should still work fine if bandwidth is
sufficient such as on LANs.

Signed-off-by: Francois Gouget 
---

Flipped the is_chunk_stride_aligned() return value.

 configure.ac   |  23 ++
 server/Makefile.am |   8 +
 server/gstreamer-encoder.c | 559 +
 server/stream.c|  18 +-
 server/video-encoder.h |   6 +-
 5 files changed, 611 insertions(+), 3 deletions(-)
 create mode 100644 server/gstreamer-encoder.c

diff --git a/configure.ac b/configure.ac
index c743875..79fe0b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,28 @@ dnl 
=
 dnl Check optional features
 SPICE_CHECK_SMARTCARD
 
+AC_ARG_ENABLE(gstreamer,
+  AS_HELP_STRING([--enable-gstreamer=@<:@auto/yes/no@:>@],
+ [Enable GStreamer 1.0 support]),,
+  [enable_gstreamer="auto"])
+
+if test "x$enable_gstreamer" != "xno"; then
+SPICE_CHECK_GSTREAMER(GSTREAMER_1_0, 1.0, [gstreamer-1.0 
gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0],
+[enable_gstreamer="yes"
+ SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-base 
1.0], [appsrc videoconvert appsink])
+ SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gstreamer-libav 
1.0], [avenc_mjpeg])
+ ],
+ [if test "x$enable_gstreamer" = "xyes"; then
+  AC_MSG_ERROR([GStreamer 1.0 support requested but not found. You 
may set GSTREAMER_1_0_CFLAGS and GSTREAMER_1_0_LIBS to avoid the need to call 
pkg-config.])
+  fi
+])
+fi
+AM_CONDITIONAL(HAVE_GSTREAMER_1_0, test "x$have_gstreamer_1_0" = "xyes")
+
+if test x"$gstreamer_missing" != x; then
+SPICE_WARNING([The following GStreamer $enable_gstreamer tools/elements 
are missing:$gstreamer_missing. The GStreamer video encoder can be built but 
may not work.])
+fi
+
 AC_ARG_ENABLE([automated_tests],
   AS_HELP_STRING([--enable-automated-tests], [Enable automated 
tests using spicy-screenshot (part of spice-gtk)]),,
   [enable_automated_tests="no"])
@@ -240,6 +262,7 @@ AC_MSG_NOTICE([
 
 LZ4 support:  ${enable_lz4}
 Smartcard:${have_smartcard}
+GStreamer 1.0:${have_gstreamer_1_0}
 SASL support: ${have_sasl}
 Automated tests:  ${enable_automated_tests}
 Manual:   ${have_asciidoc}
diff --git a/server/Makefile.am b/server/Makefile.am
index cca3b9b..ea3e2ff 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -12,6 +12,7 @@ AM_CPPFLAGS = \
$(SASL_CFLAGS)  \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
+   $(GSTREAMER_1_0_CFLAGS) \
$(SPICE_PROTOCOL_CFLAGS)\
$(SSL_CFLAGS)   \
$(VISIBILITY_HIDDEN_CFLAGS) \
@@ -45,6 +46,7 @@ libserver_la_LIBADD = 
\
$(PIXMAN_LIBS)  \
$(SASL_LIBS)\
$(SLIRP_LIBS)   \
+   $(GSTREAMER_1_0_LIBS)   \
$(SSL_LIBS) \
$(Z_LIBS)   \
$(SPICE_NONPKGCONFIG_LIBS)  \
@@ -157,6 +159,12 @@ libserver_la_SOURCES +=\
$(NULL)
 endif
 
+if HAVE_GSTREAMER_1_0
+libserver_la_SOURCES +=\
+   gstreamer-encoder.c \
+   $(NULL)
+endif
+
 libspice_server_la_LIBADD = libserver.la
 libspice_server_la_SOURCES =
 
diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
new file mode 100644
index 000..c3db28f
--- /dev/null
+++ b/server/gstreamer-encoder.c
@@ -0,0 +1,559 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2015 Jeremy White
+   Copyright (C) 2015-2016 Francois Gouget
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public

Re: [Spice-devel] [spice v15 02/21] streaming: Remove the width/height encode_frame() parameters

2016-05-30 Thread Francois Gouget 
On Fri, 27 May 2016, Christophe Fergeau wrote:

> Hey,
> 
> On Thu, May 26, 2016 at 05:15:56PM +0200, Francois Gouget wrote:
> > encode_frame() needs the QXL_DRAW_COPY operation's SpiceCopy.src_area
> > field anyway, so the width and height parameters were redundant.
> > 
> > Signed-off-by: Francois Gouget 
> > ---
> >  server/dcc-send.c  | 26 --
> >  server/mjpeg-encoder.c |  1 -
> >  server/video-encoder.h |  2 +-
> >  3 files changed, 9 insertions(+), 20 deletions(-)
> > 
> > diff --git a/server/dcc-send.c b/server/dcc-send.c
> > index 5171f9a..9ece37e 100644
> > --- a/server/dcc-send.c
> > +++ b/server/dcc-send.c
> > @@ -1660,7 +1660,6 @@ static int red_marshall_stream_data(RedChannelClient 
> > *rcc,
> >  SpiceImage *image;
> >  uint32_t frame_mm_time;
> >  uint32_t n;
> > -int width, height;
> >  int ret;
> >  
> >  if (!stream) {
> > @@ -1675,21 +1674,13 @@ static int 
> > red_marshall_stream_data(RedChannelClient *rcc,
> >  return FALSE;
> >  }
> >  
> > -if (drawable->sized_stream) {
> > -if (red_channel_client_test_remote_cap(rcc, 
> > SPICE_DISPLAY_CAP_SIZED_STREAM)) {
> > -SpiceRect *src_rect = >red_drawable->u.copy.src_area;
> > -
> > -width = src_rect->right - src_rect->left;
> > -height = src_rect->bottom - src_rect->top;
> > -} else {
> > -return FALSE;
> > -}
> > -} else {
> > -width = stream->width;
> > -height = stream->height;
> > +if (drawable->sized_stream &&
> > +!red_channel_client_test_remote_cap(rcc, 
> > SPICE_DISPLAY_CAP_SIZED_STREAM)) {
> > +return FALSE;
> >  }
> 
> I'm afraid this bit is not going to work as expected with older clients
> (ones not supporting SPICE_DISPLAY_CAP_SIZED_STREAM).

This chunk does not change whether we send a SpiceMsgDisplayStreamData 
or a SpiceMsgDisplayStreamDataSized message. Furthermore, the width / 
height we computed here are only used in the DataSized case and in 
that case we still send the frame's width / height of course:

@@ -1750,8 +1740,8 @@ static int 
red_marshall_stream_data(RedChannelClient *rcc,
 stream_data.base.id = get_stream_id(display, stream);
 stream_data.base.multi_media_time = frame_mm_time;
 stream_data.data_size = n;
-stream_data.width = width;
-stream_data.height = height;
+stream_data.width = src_area->right - src_area->left;
+stream_data.height = src_area->bottom - src_area->top;
 stream_data.dest = drawable->red_drawable->bbox;


> If we want to keep supporting the !sized_stream case, I believe we need
> both src_area, and the width/height that were used when the stream
> started.

There is no src_area in SpiceMsgDisplayStreamDataSized so it was never 
sent to the clients. Maybe you're thinking of the dest field but that's 
unchanged by this patch.



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

Re: [Spice-devel] [PATCH spice-gtk v4 3/8] spice-uri: Add missing include

2016-05-30 Thread Marc-André Lureau 
Hi

- Original Message -
> Hi Marc-André,
> 
> On Mon, 2016-05-30 at 11:54 -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> > > Related: rhbz#1335239
> > 
> > include header, but what for?
> 
> It should be included otherwise changing a declaration in spice-uri-priv.h
> will
> not require changing the corresponding definition in spice-uri.c.

Ah that makes sense now, please describe the reason in commit message too.

Forgot we had -Wmissing-prototypes...

> 
> > Shouldn't it be merged with some other patch?
> 
> Currently there is no issue. I would rather drop it than merging it to an
> unrelated patch.
> 
> Pavel
> 
> > 
> > > ---
> > >  src/spice-uri.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/src/spice-uri.c b/src/spice-uri.c
> > > index 8cf870d..3bdb502 100644
> > > --- a/src/spice-uri.c
> > > +++ b/src/spice-uri.c
> > > @@ -22,6 +22,7 @@
> > >  
> > >  #include "spice-client.h"
> > >  #include "spice-uri.h"
> > > +#include "spice-uri-priv.h"
> > >  
> > >  /**
> > >   * SECTION:spice-uri
> > > --
> > > 2.8.3
> > > 
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v4 3/8] spice-uri: Add missing include

2016-05-30 Thread Pavel Grunt 
Hi Marc-André,

On Mon, 2016-05-30 at 11:54 -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > Related: rhbz#1335239
> 
> include header, but what for?

It should be included otherwise changing a declaration in spice-uri-priv.h will
not require changing the corresponding definition in spice-uri.c.

> Shouldn't it be merged with some other patch?

Currently there is no issue. I would rather drop it than merging it to an
unrelated patch.

Pavel

> 
> > ---
> >  src/spice-uri.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/spice-uri.c b/src/spice-uri.c
> > index 8cf870d..3bdb502 100644
> > --- a/src/spice-uri.c
> > +++ b/src/spice-uri.c
> > @@ -22,6 +22,7 @@
> >  
> >  #include "spice-client.h"
> >  #include "spice-uri.h"
> > +#include "spice-uri-priv.h"
> >  
> >  /**
> >   * SECTION:spice-uri
> > --
> > 2.8.3
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v4 3/8] spice-uri: Add missing include

2016-05-30 Thread Marc-André Lureau 
Hi

- Original Message -
> Related: rhbz#1335239

include header, but what for? Shouldn't it be merged with some other patch?

> ---
>  src/spice-uri.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/spice-uri.c b/src/spice-uri.c
> index 8cf870d..3bdb502 100644
> --- a/src/spice-uri.c
> +++ b/src/spice-uri.c
> @@ -22,6 +22,7 @@
>  
>  #include "spice-client.h"
>  #include "spice-uri.h"
> +#include "spice-uri-priv.h"
>  
>  /**
>   * SECTION:spice-uri
> --
> 2.8.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-gtk v4 6/8] spice-uri: Check if port is in allowed range

2016-05-30 Thread Pavel Grunt 
Use g_ascii_strtoll because it helps to detect overflow.

Related: rhbz#1335239
---
 src/spice-uri.c| 8 ++--
 tests/test-spice-uri.c | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index ea25aaa..7eec6e5 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -170,8 +170,8 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 uri_port = uriv[1];
 
 if (uri_port != NULL) {
-char *endptr;
-guint port = strtoul(uri_port, , 10);
+gchar *endptr;
+gint64 port = g_ascii_strtoll(uri_port, , 10);
 if (*endptr != '\0') {
 g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
 "Invalid uri port: %s", uri_port);
@@ -180,6 +180,10 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar 
*_uri, GError **error)
 g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, 
"Missing uri port");
 goto end;
 }
+if (port <= 0 || port > 65535) {
+g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, 
"Port out of range");
+goto end;
+}
 spice_uri_set_port(self, port);
 }
 
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
index d1dcc59..dca2101 100644
--- a/tests/test-spice-uri.c
+++ b/tests/test-spice-uri.c
@@ -37,6 +37,8 @@ static void test_spice_uri_ipv4_bad(void)
 {"http://127.0.0.1:port;, "http", "127.0.0.1", 3128, NULL, NULL,
   "Invalid uri port: port"},
 {"http://127.0.0.1:;, "http", "127.0.0.1", 3128, NULL, NULL, "Missing 
uri port"},
+{"http://127.0.0.1:-80;, "http", "127.0.0.1", 3128, NULL, NULL, "Port 
out of range"},
+{"http://127.0.0.1:800;, "http", "127.0.0.1", 3128, NULL, NULL, 
"Port out of range"},
 };
 
 guint i;
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 7/8] spice-uri: Validate uri scheme

2016-05-30 Thread Pavel Grunt 
Related: rhbz#1335239
---
 src/spice-uri.c| 26 --
 tests/test-spice-uri.c |  2 ++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index 7eec6e5..83ebe79 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -109,7 +109,9 @@ static void spice_uri_reset(SpiceURI *self)
 G_GNUC_INTERNAL
 gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, GError **error)
 {
-gchar *dup, *uri;
+gchar *dup, *uri, **uriv = NULL;
+const gchar *uri_port = NULL;
+char *uri_scheme = NULL;
 gboolean success = FALSE;
 size_t len;
 
@@ -122,17 +124,21 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar 
*_uri, GError **error)
 uri = dup = g_strdup(_uri);
 /* FIXME: use GUri when it is ready... only support http atm */
 /* the code is voluntarily not parsing thoroughly the uri */
-if (g_ascii_strncasecmp("http://;, uri, 7) == 0) {
-uri += 7;
+uri_scheme = g_uri_parse_scheme(uri);
+if (uri_scheme == NULL) {
 spice_uri_set_scheme(self, "http");
+} else {
+spice_uri_set_scheme(self, uri_scheme);
+uri += strlen(uri_scheme) + 3; /* scheme + "://" */
+}
+if (g_ascii_strcasecmp(spice_uri_get_scheme(self), "http") == 0) {
 spice_uri_set_port(self, 3128);
-} else if (g_ascii_strncasecmp("https://;, uri, 8) == 0) {
-uri += 8;
-spice_uri_set_scheme(self, "https");
+} else if (g_ascii_strcasecmp(spice_uri_get_scheme(self), "https") == 0) {
 spice_uri_set_port(self, 3129);
 } else {
-spice_uri_set_scheme(self, "http");
-spice_uri_set_port(self, 3128);
+g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
+"Invalid uri scheme for proxy: %s", 
spice_uri_get_scheme(self));
+goto end;
 }
 /* remove trailing slash */
 len = strlen(uri);
@@ -156,8 +162,7 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 }
 
 /* max 2 parts, host:port */
-gchar **uriv = g_strsplit(uri, ":", 2);
-const gchar *uri_port = NULL;
+uriv = g_strsplit(uri, ":", 2);
 
 if (uriv[0] == NULL || strlen(uriv[0]) == 0) {
 g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
@@ -190,6 +195,7 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 success = TRUE;
 
 end:
+free(uri_scheme);
 g_free(dup);
 g_strfreev(uriv);
 return success;
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
index dca2101..51f652c 100644
--- a/tests/test-spice-uri.c
+++ b/tests/test-spice-uri.c
@@ -39,6 +39,8 @@ static void test_spice_uri_ipv4_bad(void)
 {"http://127.0.0.1:;, "http", "127.0.0.1", 3128, NULL, NULL, "Missing 
uri port"},
 {"http://127.0.0.1:-80;, "http", "127.0.0.1", 3128, NULL, NULL, "Port 
out of range"},
 {"http://127.0.0.1:800;, "http", "127.0.0.1", 3128, NULL, NULL, 
"Port out of range"},
+{"scheme://192.168.1.1:3128", "http", "127.0.0.1", 3128, NULL, NULL,
+ "Invalid uri scheme for proxy: scheme"},
 };
 
 guint i;
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 8/8] spice-uri: Add ipv6 support

2016-05-30 Thread Pavel Grunt 
Just basic support -  http://user:password@[host]:port

Resolves: rhbz#1335239
---
 src/spice-uri.c| 24 +++---
 tests/test-spice-uri.c | 90 +-
 2 files changed, 87 insertions(+), 27 deletions(-)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index 83ebe79..0376cd8 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -161,8 +161,26 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar 
*_uri, GError **error)
 uri = next;
 }
 
-/* max 2 parts, host:port */
-uriv = g_strsplit(uri, ":", 2);
+if (*uri == '[') { /* ipv6 address */
+uriv = g_strsplit(uri + 1, "]", 2);
+if (uriv[1] == NULL) {
+g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
+"Missing ']' in ipv6 uri");
+goto end;
+}
+if (*uriv[1] == ':') {
+uri_port = uriv[1] + 1;
+} else if (strlen(uriv[1]) > 0) { /* invalid string after the hostname 
*/
+g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
+"Invalid uri address");
+goto end;
+}
+} else {
+/* max 2 parts, host:port */
+uriv = g_strsplit(uri, ":", 2);
+if (uriv[0] != NULL)
+uri_port = uriv[1];
+}
 
 if (uriv[0] == NULL || strlen(uriv[0]) == 0) {
 g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
@@ -171,8 +189,6 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 }
 
 spice_uri_set_hostname(self, uriv[0]);
-if (uriv[0] != NULL)
-uri_port = uriv[1];
 
 if (uri_port != NULL) {
 gchar *endptr;
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
index 51f652c..d7aee09 100644
--- a/tests/test-spice-uri.c
+++ b/tests/test-spice-uri.c
@@ -29,26 +29,14 @@ struct test_case {
 gchar *error_msg;
 };
 
-static void test_spice_uri_ipv4_bad(void)
+static void test_spice_uri_bad(const struct test_case invalid_test_cases[], 
const guint cases_cnt)
 {
-const struct test_case invalid_test_cases[] = {
-{"http://:80;, "http", NULL, 80, NULL, NULL, "Invalid hostname in uri 
address"},
-{"http://;, "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri 
address"},
-{"http://127.0.0.1:port;, "http", "127.0.0.1", 3128, NULL, NULL,
-  "Invalid uri port: port"},
-{"http://127.0.0.1:;, "http", "127.0.0.1", 3128, NULL, NULL, "Missing 
uri port"},
-{"http://127.0.0.1:-80;, "http", "127.0.0.1", 3128, NULL, NULL, "Port 
out of range"},
-{"http://127.0.0.1:800;, "http", "127.0.0.1", 3128, NULL, NULL, 
"Port out of range"},
-{"scheme://192.168.1.1:3128", "http", "127.0.0.1", 3128, NULL, NULL,
- "Invalid uri scheme for proxy: scheme"},
-};
-
 guint i;
 
 SpiceURI *uri = spice_uri_new();
 g_assert_nonnull(uri);
 
-for (i = 0; i < G_N_ELEMENTS(invalid_test_cases); i++) {
+for (i = 0; i < cases_cnt; i++) {
 GError *error = NULL;
 g_assert_false(spice_uri_parse(uri, invalid_test_cases[i].uri, 
));
 g_assert_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED);
@@ -59,21 +47,14 @@ static void test_spice_uri_ipv4_bad(void)
 g_object_unref(uri);
 }
 
-static void test_spice_uri_ipv4_good(void)
+static void test_spice_uri_good(const struct test_case valid_test_cases[], 
const guint cases_cnt)
 {
-const struct test_case valid_test_cases[] = {
-{"http://user:password@host:80;, "http", "host", 80, "user", 
"password", NULL},
-{"http://127.0.0.1/;, "http", "127.0.0.1", 3128, NULL, NULL, NULL}, /* 
reset user & password */
-{"https://127.0.0.1;, "https", "127.0.0.1", 3129, NULL, NULL, NULL},
-{"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL, NULL},
-};
-
 guint i;
 
 SpiceURI *uri = spice_uri_new();
 g_assert_nonnull(uri);
 
-for (i = 0; i < G_N_ELEMENTS(valid_test_cases); i++) {
+for (i = 0; i < cases_cnt; i++) {
 GError *error = NULL;
 g_assert_true(spice_uri_parse(uri, valid_test_cases[i].uri, ));
 g_assert_cmpstr(spice_uri_get_scheme(uri), ==, 
valid_test_cases[i].scheme);
@@ -87,12 +68,75 @@ static void test_spice_uri_ipv4_good(void)
 g_object_unref(uri);
 }
 
+
+static void test_spice_uri_ipv4_bad(void)
+{
+const struct test_case invalid_test_cases[] = {
+{"http://:80;, "http", NULL, 80, NULL, NULL, "Invalid hostname in uri 
address"},
+{"http://;, "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri 
address"},
+{"http://127.0.0.1:port;, "http", "127.0.0.1", 3128, NULL, NULL,
+  "Invalid uri port: port"},
+{"http://127.0.0.1:;, "http", "127.0.0.1", 3128, NULL, NULL, "Missing 
uri port"},
+{"http://127.0.0.1:-80;, "http", "127.0.0.1", 3128, NULL, NULL, "Port 
out of range"},
+{"http://127.0.0.1:800;, "http", "127.0.0.1",

[Spice-devel] [PATCH spice-gtk v4 5/8] spice-uri: Do not allow empty port string

2016-05-30 Thread Pavel Grunt 
Related: rhbz#1335239
---
 src/spice-uri.c| 3 +++
 tests/test-spice-uri.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index 9f793de..ea25aaa 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -176,6 +176,9 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
 "Invalid uri port: %s", uri_port);
 goto end;
+} else if (endptr == uri_port) {
+g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, 
"Missing uri port");
+goto end;
 }
 spice_uri_set_port(self, port);
 }
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
index e177723..d1dcc59 100644
--- a/tests/test-spice-uri.c
+++ b/tests/test-spice-uri.c
@@ -36,6 +36,7 @@ static void test_spice_uri_ipv4_bad(void)
 {"http://;, "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri 
address"},
 {"http://127.0.0.1:port;, "http", "127.0.0.1", 3128, NULL, NULL,
   "Invalid uri port: port"},
+{"http://127.0.0.1:;, "http", "127.0.0.1", 3128, NULL, NULL, "Missing 
uri port"},
 };
 
 guint i;
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 2/8] spice-uri: Mark parameter as unused

2016-05-30 Thread Pavel Grunt 
---
 src/spice-uri.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index e1317bd..8cf870d 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -340,7 +340,7 @@ static void spice_uri_finalize(GObject* obj)
 G_OBJECT_CLASS (spice_uri_parent_class)->finalize (obj);
 }
 
-static void spice_uri_init (SpiceURI *self)
+static void spice_uri_init (SpiceURI *self G_GNUC_UNUSED)
 {
 }
 
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 3/8] spice-uri: Add missing include

2016-05-30 Thread Pavel Grunt 
Related: rhbz#1335239
---
 src/spice-uri.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index 8cf870d..3bdb502 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -22,6 +22,7 @@
 
 #include "spice-client.h"
 #include "spice-uri.h"
+#include "spice-uri-priv.h"
 
 /**
  * SECTION:spice-uri
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 4/8] spice-uri: Reset SpiceUri before parsing

2016-05-30 Thread Pavel Grunt 
Avoid using old values after parsing a new uri.

Related: rhbz#1335239
---
 src/spice-uri.c| 17 +
 tests/test-spice-uri.c |  4 ++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/spice-uri.c b/src/spice-uri.c
index 3bdb502..9f793de 100644
--- a/src/spice-uri.c
+++ b/src/spice-uri.c
@@ -97,6 +97,15 @@ SpiceURI* spice_uri_new(void)
 return self;
 }
 
+static void spice_uri_reset(SpiceURI *self)
+{
+g_clear_pointer(>scheme, g_free);
+g_clear_pointer(>hostname, g_free);
+g_clear_pointer(>user, g_free);
+g_clear_pointer(>password, g_free);
+self->port = 0;
+}
+
 G_GNUC_INTERNAL
 gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, GError **error)
 {
@@ -105,6 +114,9 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar *_uri, 
GError **error)
 size_t len;
 
 g_return_val_if_fail(self != NULL, FALSE);
+
+spice_uri_reset(self);
+
 g_return_val_if_fail(_uri != NULL, FALSE);
 
 uri = dup = g_strdup(_uri);
@@ -333,10 +345,7 @@ static void spice_uri_finalize(GObject* obj)
 SpiceURI *self;
 
 self = G_TYPE_CHECK_INSTANCE_CAST(obj, SPICE_TYPE_URI, SpiceURI);
-g_free(self->scheme);
-g_free(self->hostname);
-g_free(self->user);
-g_free(self->password);
+spice_uri_reset(self);
 
 G_OBJECT_CLASS (spice_uri_parent_class)->finalize (obj);
 }
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
index 00aeea7..e177723 100644
--- a/tests/test-spice-uri.c
+++ b/tests/test-spice-uri.c
@@ -57,10 +57,10 @@ static void test_spice_uri_ipv4_bad(void)
 static void test_spice_uri_ipv4_good(void)
 {
 const struct test_case valid_test_cases[] = {
-{"http://127.0.0.1/;, "http", "127.0.0.1", 3128, NULL, NULL, NULL},
+{"http://user:password@host:80;, "http", "host", 80, "user", 
"password", NULL},
+{"http://127.0.0.1/;, "http", "127.0.0.1", 3128, NULL, NULL, NULL}, /* 
reset user & password */
 {"https://127.0.0.1;, "https", "127.0.0.1", 3129, NULL, NULL, NULL},
 {"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL, NULL},
-{"http://user:password@host:80;, "http", "host", 80, "user", 
"password", NULL},
 };
 
 guint i;
-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 0/8] Allow ipv6 proxy url

2016-05-30 Thread Pavel Grunt 
Hi,

these patches add ipv6 support to SpiceUri, so it can be used in SPICE_PROXY.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1335239

v4 contains changes suggested by Christophe, Victor, Frediano:
 - Added check for scheme
 - Tests check for error messages
 - Port 0 is considered invalid
 - Fixed memory leaks

v3 per Frediano's review:
 - Added test for port out of range (0, 65535)
 - reset all values of SpiceUri before parsing

v2 contains changes suggested by Frediano and Victor:
 - more tests (missing port, missing ending bracket)
 - tests are defined in array, so it is easier to add new test cases
 - follow ipv6 address format http://user:password@[ipv6]:port

Thanks,

Pavel Grunt (8):
  tests: Add test for SpiceUri
  spice-uri: Mark parameter as unused
  spice-uri: Add missing include
  spice-uri: Reset SpiceUri before parsing
  spice-uri: Do not allow empty port string
  spice-uri: Check if port is in allowed range
  spice-uri: Validate uri scheme
  spice-uri: Add ipv6 support

 src/spice-uri.c|  79 ---
 tests/Makefile.am  |   2 +
 tests/test-spice-uri.c | 142 +
 3 files changed, 203 insertions(+), 20 deletions(-)
 create mode 100644 tests/test-spice-uri.c

-- 
2.8.3

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

[Spice-devel] [PATCH spice-gtk v4 1/8] tests: Add test for SpiceUri

2016-05-30 Thread Pavel Grunt 
Related: rhbz#1335239
---
 tests/Makefile.am  |  2 ++
 tests/test-spice-uri.c | 93 ++
 2 files changed, 95 insertions(+)
 create mode 100644 tests/test-spice-uri.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c1d95c1..fb97138 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -4,6 +4,7 @@ noinst_PROGRAMS =
 TESTS = coroutine  \
util\
session \
+   test-spice-uri  \
$(NULL)
 
 if WITH_PHODAV
@@ -35,6 +36,7 @@ util_SOURCES = util.c
 coroutine_SOURCES = coroutine.c
 session_SOURCES = session.c
 pipe_SOURCES = pipe.c
+test_spice_uri_SOURCES = test-spice-uri.c
 usb_acl_helper_SOURCES = usb-acl-helper.c
 usb_acl_helper_CFLAGS = -DTESTDIR=\"$(abs_builddir)\"
 mock_acl_helper_SOURCES = mock-acl-helper.c
diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
new file mode 100644
index 000..00aeea7
--- /dev/null
+++ b/tests/test-spice-uri.c
@@ -0,0 +1,93 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2016 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+#include 
+#include 
+#include "spice-uri-priv.h"
+
+struct test_case {
+gchar *uri;
+gchar *scheme;
+gchar *hostname;
+guint port;
+gchar *user;
+gchar *password;
+gchar *error_msg;
+};
+
+static void test_spice_uri_ipv4_bad(void)
+{
+const struct test_case invalid_test_cases[] = {
+{"http://:80;, "http", NULL, 80, NULL, NULL, "Invalid hostname in uri 
address"},
+{"http://;, "http", NULL, 3128, NULL, NULL, "Invalid hostname in uri 
address"},
+{"http://127.0.0.1:port;, "http", "127.0.0.1", 3128, NULL, NULL,
+  "Invalid uri port: port"},
+};
+
+guint i;
+
+SpiceURI *uri = spice_uri_new();
+g_assert_nonnull(uri);
+
+for (i = 0; i < G_N_ELEMENTS(invalid_test_cases); i++) {
+GError *error = NULL;
+g_assert_false(spice_uri_parse(uri, invalid_test_cases[i].uri, 
));
+g_assert_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED);
+g_assert_cmpstr(error->message, ==, invalid_test_cases[i].error_msg);
+g_error_free(error);
+}
+
+g_object_unref(uri);
+}
+
+static void test_spice_uri_ipv4_good(void)
+{
+const struct test_case valid_test_cases[] = {
+{"http://127.0.0.1/;, "http", "127.0.0.1", 3128, NULL, NULL, NULL},
+{"https://127.0.0.1;, "https", "127.0.0.1", 3129, NULL, NULL, NULL},
+{"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL, NULL},
+{"http://user:password@host:80;, "http", "host", 80, "user", 
"password", NULL},
+};
+
+guint i;
+
+SpiceURI *uri = spice_uri_new();
+g_assert_nonnull(uri);
+
+for (i = 0; i < G_N_ELEMENTS(valid_test_cases); i++) {
+GError *error = NULL;
+g_assert_true(spice_uri_parse(uri, valid_test_cases[i].uri, ));
+g_assert_cmpstr(spice_uri_get_scheme(uri), ==, 
valid_test_cases[i].scheme);
+g_assert_cmpstr(spice_uri_get_hostname(uri), ==, 
valid_test_cases[i].hostname);
+g_assert_cmpstr(spice_uri_get_user(uri), ==, valid_test_cases[i].user);
+g_assert_cmpstr(spice_uri_get_password(uri), ==, 
valid_test_cases[i].password);
+g_assert_cmpuint(spice_uri_get_port(uri), ==, 
valid_test_cases[i].port);
+g_assert_no_error(error);
+}
+
+g_object_unref(uri);
+}
+
+int main(int argc, char* argv[])
+{
+g_test_init(, , NULL);
+
+g_test_add_func("/spice_uri/ipv4/bad-uri", test_spice_uri_ipv4_bad);
+g_test_add_func("/spice_uri/ipv4/good-uri", test_spice_uri_ipv4_good);
+
+return g_test_run();
+}
-- 
2.8.3

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

Re: [Spice-devel] [spice-gtk v3 00/16] separate SpiceFileTransferTask logic from channel-main

2016-05-30 Thread Victor Toso 
Hi,

One could test this with the following branch:
https://cgit.freedesktop.org/~victortoso/spice-gtk/log/?h=move-xfer

Cheers,
  toso

On Mon, May 30, 2016 at 11:54:56AM +0200, Victor Toso wrote:
> v2->v3
> * rebased
> * pushed v2 01/16 ~ 349a52ca2d6af4 - acked by Pavel and Jonathon
> * split v2 02/16 to have private helpers in different patch (Pavel)
> - v3 01/16 ~ file-xfer: get functions for SpiceFileTransferTask
> - v3 02/16 ~ file-xfer: introduce flush_callback and flush_done
> * change assert to return-if-fail in private helpers (Pavel)
> * created helper function to get task id (instead of using
>   a lot of g_object_get)
> * removed CHANNEL_DEBUG check for NULL channel and changed
>   the code to use SPICE_DEBUG instead (Pavel)
> * include a fix to the progress info which was not taking in
>   consideration transfers that were cancelled
> 
> v2:
> > The idea is mainly to have a design improvement over the current
> > file-transfer logic by trying to detach it from channel-main and the
> > agent logic plus a few simple tests.
> >
> > One interesting change is about creating a FileTransfereOperation per
> > drag-and-drop operation in channel-main, which removes the
> > progress_callback logic from SpiceFileTransferTask as we don't want to
> > call progress_callback on SpiceFileTransferTask but rather on
> > channel-main;
> >
> > We are also calling the application callback for each finalized
> > SpiceFileTransferTask which means that it could be several callback
> > calls per operation. I suggested to fix that on patch 07.
> >
> > Let me know your thoughts about this.
> >   Thanks
> 
> Victor Toso (16):
>   file-xfer: get functions for SpiceFileTransferTask
>   file-xfer: introduce flush_callback and flush_done
>   file-xfer: introduce create_tasks and start_task
>   file-xfer: introduce file-info signal
>   file-xfer: inform agent of errors only when task finished
>   file-xfer: a FileTransferOperation per transfer call
>   file-xfer: call user callback once per operation
>   file-xfer: fix progress info on cancelled transfers
>   file-xfer: move to spice-file-transfer-task.c
>   tests: fix build with smartcard enabled
>   tests: file-transfer include simple tests
>   tests: file-transfer cancel on task start
>   channel: avoid crash on spice_channel_wakeup due NULL channel
>   tests: file-transfer cancel on file-info
>   tests: file-transfer cancel on read file
>   tests: file-transfer agent send error/cancel
> 
>  src/Makefile.am |   2 +
>  src/channel-main.c  | 896 
> +++-
>  src/spice-channel.c |   5 +-
>  src/spice-file-transfer-task-priv.h |  58 ++
>  src/spice-file-transfer-task.c  | 713 +++
>  src/tmp-introspect325cwcm0/SpiceClientGtk-3.0.c | 628 +
>  tests/Makefile.am   |   3 +
>  tests/file-transfer.c   | 412 +++
>  8 files changed, 2054 insertions(+), 663 deletions(-)
>  create mode 100644 src/spice-file-transfer-task-priv.h
>  create mode 100644 src/spice-file-transfer-task.c
>  create mode 100644 src/tmp-introspect325cwcm0/SpiceClientGtk-3.0.c
>  create mode 100644 tests/file-transfer.c
> 
> -- 
> 2.5.5
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v3 15/16] tests: file-transfer cancel on read file

2016-05-30 Thread Victor Toso 
---
 tests/file-transfer.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/tests/file-transfer.c b/tests/file-transfer.c
index d8dcf6c..be4d585 100644
--- a/tests/file-transfer.c
+++ b/tests/file-transfer.c
@@ -211,6 +211,47 @@ test_cancel_on_file_info(Fixture *f, gconstpointer 
user_data)
 g_main_loop_run (f->loop);
 }
 
+/***
+ * TEST CANCEL ON READ FILE
+ 
**/
+
+static void
+transfer_flush_callback_cancel_after_flush(SpiceFileTransferTask *xfer_task,
+   void *buffer,
+   gssize count,
+   gpointer user_data)
+{
+gint i;
+Fixture *f = user_data;
+for (i = 0; i < f->num_files; i++) {
+g_test_expect_message("GSpice", G_LOG_LEVEL_CRITICAL, "*assertion 
'channel != NULL'*");
+}
+spice_file_transfer_task_flush_done(xfer_task, NULL);
+g_cancellable_cancel(f->cancellable);
+agent_send_success_async(xfer_task);
+}
+
+static void
+test_cancel_on_read_file(Fixture *f, gconstpointer user_data)
+{
+GList *it;
+
+f->tasks = spice_file_transfer_task_create_tasks(NULL,
+ f->files,
+ G_FILE_COPY_NONE,
+ f->cancellable,
+ 
transfer_flush_callback_cancel_after_flush,
+ f,
+ transfer_done,
+ f);
+for (it = f->tasks; it != NULL; it = it->next) {
+SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(it->data);
+g_signal_connect(xfer_task, "file-info", 
G_CALLBACK(transfer_task_on_file_info), f);
+spice_file_transfer_task_start_task(xfer_task);
+}
+g_main_loop_run (f->loop);
+}
+
 /* Tests summary:
  *
  * This tests are specific to SpiceFileTransferTask and how it handles the
@@ -257,6 +298,10 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(SINGLE_FILE),
f_setup, test_cancel_on_file_info, f_teardown);
 
+g_test_add("/spice-file-transfer-task/single/cancel/on-read-file",
+   Fixture, GUINT_TO_POINTER(SINGLE_FILE),
+   f_setup, test_cancel_on_read_file, f_teardown);
+
 g_test_add("/spice-file-transfer-task/multiple/simple-transfer",
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_simple_transfer, f_teardown);
@@ -269,5 +314,9 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_cancel_on_file_info, f_teardown);
 
+g_test_add("/spice-file-transfer-task/multiple/cancel/on-read-file",
+   Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
+   f_setup, test_cancel_on_read_file, f_teardown);
+
 return g_test_run();
 }
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 08/16] file-xfer: fix progress info on cancelled transfers

2016-05-30 Thread Victor Toso 
Application can start multiple file-transfers in one operation and
cancel a few of them while the operation is ongoing. In that case, we
should remove the file-size of the transfer operation otherwise we
will send incorrect progress data.

Taking in consideration the split of SpiceFileTransferTask, this patch
includes spice_file_transfer_task_get_file_size() internal helper
function.
---
 src/channel-main.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index e204a1e..890d939 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -61,6 +61,7 @@ typedef void 
(*SpiceFileTransferTaskFlushCb)(SpiceFileTransferTask *xfer_task,
 static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
 static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
 static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
+static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask 
*self);
 static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self, 
GError *error);
 static GList *spice_file_transfer_task_create_tasks(SpiceMainChannel *channel,
 GFile **files,
@@ -1927,6 +1928,7 @@ static void file_xfer_end_callback(GObject *source_object,
 {
 GTask *task;
 FileTransferOperation *xfer_op;
+SpiceFileTransferTask *xfer_task;
 
 task = G_TASK(res);
 if (!g_task_had_error(task))
@@ -1934,7 +1936,9 @@ static void file_xfer_end_callback(GObject *source_object,
  * file_transfer_operation_task_finished */
 return;
 
+xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
 xfer_op = user_data;
+xfer_op->transfer_size -= 
spice_file_transfer_task_get_file_size(xfer_task);
 
 if (xfer_op->error != NULL)
 return;
@@ -1948,9 +1952,6 @@ static void file_xfer_end_callback(GObject *source_object,
  * without GCancellabe */
 if (g_error_matches(xfer_op->error, G_IO_ERROR, G_IO_ERROR_CANCELLED) &&
 xfer_op->cancellable == NULL) {
-SpiceFileTransferTask *xfer_task;
-
-xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
 spice_debug("file-transfer %u was cancelled",
 spice_file_transfer_task_get_id(xfer_task));
 g_clear_error(_op->error);
@@ -3382,6 +3383,12 @@ static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferT
 return self->cancellable;
 }
 
+static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask 
*self)
+{
+g_return_val_if_fail(self != NULL, 0);
+return self->file_size;
+}
+
 static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self, 
GError *error)
 {
 g_return_if_fail(self != NULL);
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 07/16] file-xfer: call user callback once per operation

2016-05-30 Thread Victor Toso 
SpiceFileTransferTask has a callback to be called when operation
ended. Til this patch, we were setting the user callback which means
that in multiple file-transfers, we were calling the user callback
several times.

Following the same logic pointed from 113093dd00a1cf10f6d3c3589b7 this
is a SpiceMainChannel operation and it should only call the user
callback when this operation is over (FileTransferOperation now).
---
 src/channel-main.c  |  72 ++-
 src/tmp-introspect325cwcm0/SpiceClientGtk-3.0.c | 628 
 2 files changed, 694 insertions(+), 6 deletions(-)
 create mode 100644 src/tmp-introspect325cwcm0/SpiceClientGtk-3.0.c

diff --git a/src/channel-main.c b/src/channel-main.c
index f36326d..e204a1e 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -157,6 +157,10 @@ typedef struct {
 SpiceMainChannel   *channel;
 GFileProgressCallback   progress_callback;
 gpointerprogress_callback_data;
+GAsyncReadyCallback end_callback;
+gpointerend_callback_data;
+GError *error;
+GCancellable   *cancellable;
 goffset total_sent;
 goffset transfer_size;
 } FileTransferOperation;
@@ -1838,9 +1842,7 @@ static void file_xfer_close_cb(GObject  *object,
 }
 }
 
-/* Notify to user that files have been transferred or something error
-   happened. */
-task = g_task_new(self->channel,
+task = g_task_new(self,
   self->cancellable,
   self->callback,
   self->user_data);
@@ -1919,6 +1921,42 @@ static void 
file_xfer_flush_callback(SpiceFileTransferTask *xfer_task,
 file_xfer_flush_async(main_channel, cancellable, 
file_xfer_data_flushed_cb, xfer_task);
 }
 
+static void file_xfer_end_callback(GObject *source_object,
+   GAsyncResult *res,
+   gpointer user_data)
+{
+GTask *task;
+FileTransferOperation *xfer_op;
+
+task = G_TASK(res);
+if (!g_task_had_error(task))
+/* SpiceFileTransferTask and FileTransferOperation are freed on
+ * file_transfer_operation_task_finished */
+return;
+
+xfer_op = user_data;
+
+if (xfer_op->error != NULL)
+return;
+
+/* Get the GError from SpiceFileTransferTask so we can properly return to
+ * the application when the FileTransferOperation ends */
+g_task_propagate_boolean(task, _op->error);
+
+/* User can cancel a FileTransfer without cancelling the whole
+ * operation. For that, spice_main_file_copy_async must be called
+ * without GCancellabe */
+if (g_error_matches(xfer_op->error, G_IO_ERROR, G_IO_ERROR_CANCELLED) &&
+xfer_op->cancellable == NULL) {
+SpiceFileTransferTask *xfer_task;
+
+xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
+spice_debug("file-transfer %u was cancelled",
+spice_file_transfer_task_get_id(xfer_task));
+g_clear_error(_op->error);
+}
+}
+
 /* main context */
 static void file_xfer_read_cb(GObject *source_object,
   GAsyncResult *res,
@@ -3108,10 +3146,24 @@ static void 
file_transfer_operation_end(FileTransferOperation *xfer_op)
 g_return_if_fail(xfer_op != NULL);
 spice_debug("Freeing file-transfer-operation %p", xfer_op);
 
+if (xfer_op->end_callback) {
+GTask *task = g_task_new(xfer_op->channel,
+ xfer_op->cancellable,
+ xfer_op->end_callback,
+ xfer_op->end_callback_data);
+
+if (xfer_op->error != NULL) {
+g_task_return_error(task, xfer_op->error);
+} else {
+g_task_return_boolean(task, TRUE);
+}
+}
+
 /* SpiceFileTransferTask itself is freed after it emits "finish" */
 if (xfer_op->tasks != NULL)
 g_list_free(xfer_op->tasks);
 
+g_clear_object (_op->cancellable);
 g_free(xfer_op);
 }
 
@@ -3225,7 +3277,11 @@ static void task_finished(SpiceFileTransferTask *task,
  * files, please connect to the #SpiceMainChannel::new-file-transfer signal.
  *
  * When the operation is finished, callback will be called. You can then call
- * spice_main_file_copy_finish() to get the result of the operation.
+ * spice_main_file_copy_finish() to get the result of the operation. Note that
+ * before release 0.32 the callback was called for each file in multiple file
+ * transfer. This behavior was changed for the same reason as the
+ * progress_callback (above). If you need to monitor the ending of individual
+ * files, you can connect to "finished" signal from each SpiceFileTransferTask.
  *
  **/
 void spice_main_file_copy_async(SpiceMainChannel *channel,
@@ -3259,15 +3315,19 @@ void spice_main_file_copy_async(SpiceMainChannel 
*channel,

[Spice-devel] [spice-gtk v3 11/16] tests: file-transfer include simple tests

2016-05-30 Thread Victor Toso 
This only includes a simple test for file-transfer with a small
summary of the possible situations of the test.

As the test is specifically for SpiceFileTransferTask, we don't create
a SpiceMainChannel. That could cause a simple crash on CHANNEL_DEBUG
which this patch addresses.
---
 tests/Makefile.am |   2 +
 tests/file-transfer.c | 190 ++
 2 files changed, 192 insertions(+)
 create mode 100644 tests/file-transfer.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d56ff1b..56f6c06 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -4,6 +4,7 @@ noinst_PROGRAMS =
 TESTS = coroutine  \
util\
session \
+   test-file-transfer  \
$(NULL)
 
 if WITH_PHODAV
@@ -36,6 +37,7 @@ util_SOURCES = util.c
 coroutine_SOURCES = coroutine.c
 session_SOURCES = session.c
 pipe_SOURCES = pipe.c
+test_file_transfer_SOURCES = file-transfer.c
 usb_acl_helper_SOURCES = usb-acl-helper.c
 usb_acl_helper_CFLAGS = -DTESTDIR=\"$(abs_builddir)\"
 mock_acl_helper_SOURCES = mock-acl-helper.c
diff --git a/tests/file-transfer.c b/tests/file-transfer.c
new file mode 100644
index 000..5ef11a7
--- /dev/null
+++ b/tests/file-transfer.c
@@ -0,0 +1,190 @@
+#include 
+
+#include "spice-file-transfer-task-priv.h"
+
+typedef struct _Fixture {
+GFile **files;
+guint   num_files;
+guint   num_files_done;
+GCancellable   *cancellable;
+GMainLoop  *loop;
+GList  *tasks;
+} Fixture;
+
+typedef struct _AgentAsync {
+SpiceFileTransferTask  *xfer_task;
+VDAgentFileXferStatusMessagemsg;
+} AgentAsync;
+
+#define SINGLE_FILE 1
+#define MULTIPLE_FILES  10
+
+#define T10ns (G_TIME_SPAN_MILLISECOND / 100)
+
+const gchar content[] = "0123456789_spice-file-transfer-task";
+
+static void
+f_setup(Fixture *f, gconstpointer user_data)
+{
+gint i;
+GError *err = NULL;
+
+f->loop = g_main_loop_new(NULL, FALSE);
+f->num_files = GPOINTER_TO_UINT(user_data);
+f->num_files_done = 0;
+f->files = g_new0(GFile *, f->num_files + 1);
+f->cancellable = g_cancellable_new();
+for (i = 0; i < f->num_files; i++) {
+gboolean success;
+GFileIOStream *iostream;
+
+f->files[i] = g_file_new_tmp("spice-file-transfer-XX", , 
);
+g_assert_no_error(err);
+g_assert_nonnull(iostream);
+g_clear_object();
+
+success = g_file_replace_contents (f->files[i], content, 
strlen(content), NULL, FALSE,
+   G_FILE_CREATE_NONE, NULL, 
f->cancellable, );
+g_assert_no_error(err);
+g_assert_true(success);
+}
+}
+
+static void
+f_teardown(Fixture *f, gconstpointer user_data)
+{
+gint i;
+GError *err = NULL;
+
+g_main_loop_unref(f->loop);
+g_clear_object(>cancellable);
+g_clear_pointer(>tasks, g_list_free);
+
+for (i = 0; i < f->num_files; i++) {
+g_file_delete(f->files[i], NULL, );
+g_assert_no_error(err);
+g_object_unref(f->files[i]);
+}
+g_clear_pointer(>files, g_free);
+}
+
+static gboolean
+agent_send_msg(gpointer user_data)
+{
+AgentAsync *aa = user_data;
+spice_file_transfer_task_handle_status(aa->xfer_task, >msg);
+g_free(aa);
+return G_SOURCE_REMOVE;
+}
+
+static void
+agent_send_msg_async(SpiceFileTransferTask *xfer_task, guint32 result)
+{
+AgentAsync *aa = g_new0(AgentAsync, 1);
+aa->xfer_task = xfer_task;
+aa->msg.result = result;
+g_object_get(xfer_task, "id", >msg.id, NULL);
+g_timeout_add_full(G_PRIORITY_LOW, T10ns, agent_send_msg, aa, NULL);
+}
+
+#define agent_send_success_async(t)   agent_send_msg_async(t, 
VD_AGENT_FILE_XFER_STATUS_SUCCESS)
+#define agent_send_more_data_async(t) agent_send_msg_async(t, 
VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA)
+#define agent_send_cancel_async(t)agent_send_msg_async(t, 
VD_AGENT_FILE_XFER_STATUS_CANCELLED)
+#define agent_send_error_async(t) agent_send_msg_async(t, 
VD_AGENT_FILE_XFER_STATUS_ERROR)
+
+/***
+ * TEST SIMPLE TRANSFER
+ 
**/
+static void
+transfer_done(GObject *source_object, GAsyncResult *res, gpointer user_data)
+{
+Fixture *f = user_data;
+
+f->num_files_done++;
+if (f->num_files == f->num_files_done)
+g_main_loop_quit(f->loop);
+}
+
+static void
+transfer_flush_callback(SpiceFileTransferTask *xfer_task,
+void *buffer,
+gssize count,
+gpointer user_data)
+{
+spice_file_transfer_task_flush_done(xfer_task, NULL);
+agent_send_success_async(xfer_task);
+}
+
+static void
+transfer_task_on_file_info(SpiceFileTransferTask *xfer_task, GFileInfo *info, 
gpointer

[Spice-devel] [spice-gtk v3 13/16] channel: avoid crash on spice_channel_wakeup due NULL channel

2016-05-30 Thread Victor Toso 
---
 src/spice-channel.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index c555f75..8b159f4 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1891,7 +1891,10 @@ error:
 G_GNUC_INTERNAL
 void spice_channel_wakeup(SpiceChannel *channel, gboolean cancel)
 {
-GCoroutine *c = >priv->coroutine;
+GCoroutine *c;
+
+g_return_if_fail (channel != NULL);
+c = >priv->coroutine;
 
 if (cancel)
 g_coroutine_condition_cancel(c);
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 09/16] file-xfer: move to spice-file-transfer-task.c

2016-05-30 Thread Victor Toso 
Previous six patches are related to this change. This patch moves:
* GObject boilerplate
* External API related to SpiceFileTransferTask
* Internal API needed by channel-main
* Helpers that belong to this object
---
 src/Makefile.am |   2 +
 src/channel-main.c  | 693 +--
 src/spice-file-transfer-task-priv.h |  58 +++
 src/spice-file-transfer-task.c  | 713 
 4 files changed, 774 insertions(+), 692 deletions(-)
 create mode 100644 src/spice-file-transfer-task-priv.h
 create mode 100644 src/spice-file-transfer-task.c

diff --git a/src/Makefile.am b/src/Makefile.am
index 73bb39c..35ac906 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -233,6 +233,8 @@ libspice_client_glib_2_0_la_SOURCES =   
\
spice-channel.c \
spice-channel-cache.h   \
spice-channel-priv.h\
+   spice-file-transfer-task.c  \
+   spice-file-transfer-task-priv.h \
coroutine.h \
gio-coroutine.c \
gio-coroutine.h \
diff --git a/src/channel-main.c b/src/channel-main.c
index 890d939..9518323 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -29,7 +29,7 @@
 #include "spice-channel-priv.h"
 #include "spice-session-priv.h"
 #include "spice-audio-priv.h"
-#include "spice-file-transfer-task.h"
+#include "spice-file-transfer-task-priv.h"
 
 /**
  * SECTION:channel-main
@@ -54,91 +54,6 @@
 
 typedef struct spice_migrate spice_migrate;
 
-typedef void (*SpiceFileTransferTaskFlushCb)(SpiceFileTransferTask *xfer_task,
- void *buffer,
- gssize count,
- gpointer user_data);
-static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
-static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
-static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
-static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask 
*self);
-static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self, 
GError *error);
-static GList *spice_file_transfer_task_create_tasks(SpiceMainChannel *channel,
-GFile **files,
-GFileCopyFlags flags,
-GCancellable *cancellable,
-
SpiceFileTransferTaskFlushCb flush_callback,
-gpointer 
flush_callback_data,
-GAsyncReadyCallback 
callback,
-gpointer user_data);
-static void spice_file_transfer_task_start_task(SpiceFileTransferTask *self);
-
-/**
- * SECTION:file-transfer-task
- * @short_description: Monitoring file transfers
- * @title: File Transfer Task
- * @section_id:
- * @see_also: #SpiceMainChannel
- * @stability: Stable
- * @include: spice-client.h
- *
- * SpiceFileTransferTask is an object that represents a particular file
- * transfer between the client and the guest. The properties and signals of the
- * object can be used to monitor the status and result of the transfer. The
- * Main Channel's #SpiceMainChannel::new-file-transfer signal will be emitted
- * whenever a new file transfer task is initiated.
- *
- * Since: 0.31
- */
-
-struct _SpiceFileTransferTask
-{
-GObject parent;
-
-uint32_t   id;
-gboolean   pending;
-GFile  *file;
-SpiceMainChannel   *channel;
-GFileInputStream   *file_stream;
-GFileCopyFlags flags;
-GCancellable   *cancellable;
-SpiceFileTransferTaskFlushCb   flush_callback;
-gpointer   flush_callback_data;
-GAsyncReadyCallbackcallback;
-gpointer   user_data;
-char   *buffer;
-uint64_t   read_bytes;
-uint64_t   file_size;
-gint64 start_time;
-gint64 last_update;
-GError *error;
-};
-
-struct _SpiceFileTransferTaskClass
-{
-GObjectClass parent_class;
-};
-
-G_DEFINE_TYPE(SpiceFileTransferTask, spice_file_transfer_task, G_TYPE_OBJECT)
-
-#define FILE_XFER_CHUNK_SIZE (VD_AGENT_MAX_DATA_SIZE * 32)
-
-enum {
-PROP_TASK_ID = 1,
-PROP_TASK_CHANNEL,
-PROP_TASK_CANCELLABLE,
-PROP_TASK_FILE,
-PROP_TASK_PROGRESS,
-};
-

[Spice-devel] [spice-gtk v3 12/16] tests: file-transfer cancel on task start

2016-05-30 Thread Victor Toso 
---
 tests/file-transfer.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/tests/file-transfer.c b/tests/file-transfer.c
index 5ef11a7..1421f1e 100644
--- a/tests/file-transfer.c
+++ b/tests/file-transfer.c
@@ -144,6 +144,32 @@ test_simple_transfer(Fixture *f, gconstpointer user_data)
 g_main_loop_run (f->loop);
 }
 
+/***
+ * TEST CANCEL ON START TASK
+ 
**/
+
+static void
+test_cancel_on_start_task(Fixture *f, gconstpointer user_data)
+{
+GList *it;
+
+f->tasks = spice_file_transfer_task_create_tasks(NULL,
+ f->files,
+ G_FILE_COPY_NONE,
+ f->cancellable,
+ transfer_flush_callback,
+ NULL,
+ transfer_done,
+ f);
+for (it = f->tasks; it != NULL; it = it->next) {
+SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(it->data);
+g_signal_connect(xfer_task, "file-info", 
G_CALLBACK(transfer_task_on_file_info), f);
+g_cancellable_cancel(f->cancellable);
+spice_file_transfer_task_start_task(xfer_task);
+}
+g_main_loop_run (f->loop);
+}
+
 /* Tests summary:
  *
  * This tests are specific to SpiceFileTransferTask and how it handles the
@@ -182,9 +208,17 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(SINGLE_FILE),
f_setup, test_simple_transfer, f_teardown);
 
+g_test_add("/spice-file-transfer-task/single/cancel/on-start-task",
+   Fixture, GUINT_TO_POINTER(SINGLE_FILE),
+   f_setup, test_cancel_on_start_task, f_teardown);
+
 g_test_add("/spice-file-transfer-task/multiple/simple-transfer",
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_simple_transfer, f_teardown);
 
+g_test_add("/spice-file-transfer-task/multiple/cancel/on-start-task",
+   Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
+   f_setup, test_cancel_on_start_task, f_teardown);
+
 return g_test_run();
 }
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 16/16] tests: file-transfer agent send error/cancel

2016-05-30 Thread Victor Toso 
Agent only can only send error or cancel from a transfer operation
after it was initialized. In the context of SpiceFileTransferTask, it
means that we need to test only after file-info was emitted.
---
 tests/file-transfer.c | 90 +++
 1 file changed, 90 insertions(+)

diff --git a/tests/file-transfer.c b/tests/file-transfer.c
index be4d585..23f7d42 100644
--- a/tests/file-transfer.c
+++ b/tests/file-transfer.c
@@ -252,6 +252,80 @@ test_cancel_on_read_file(Fixture *f, gconstpointer 
user_data)
 g_main_loop_run (f->loop);
 }
 
+/***
+ * TEST AGENT CANCEL ON READ
+ 
**/
+
+static void
+transfer_flush_callback_agent_cancel(SpiceFileTransferTask *xfer_task,
+ void *buffer,
+ gssize count,
+ gpointer user_data)
+{
+gint i;
+Fixture *f = user_data;
+spice_file_transfer_task_flush_done(xfer_task, NULL);
+agent_send_cancel_async(xfer_task);
+}
+
+static void
+test_agent_cancel_on_read(Fixture *f, gconstpointer user_data)
+{
+GList *it;
+
+f->tasks = spice_file_transfer_task_create_tasks(NULL,
+ f->files,
+ G_FILE_COPY_NONE,
+ f->cancellable,
+ 
transfer_flush_callback_agent_cancel,
+ f,
+ transfer_done,
+ f);
+for (it = f->tasks; it != NULL; it = it->next) {
+SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(it->data);
+g_signal_connect(xfer_task, "file-info", 
G_CALLBACK(transfer_task_on_file_info), f);
+spice_file_transfer_task_start_task(xfer_task);
+}
+g_main_loop_run (f->loop);
+}
+
+/***
+ * TEST AGENT ERROR ON READ
+ 
**/
+
+static void
+transfer_flush_callback_agent_error(SpiceFileTransferTask *xfer_task,
+void *buffer,
+gssize count,
+gpointer user_data)
+{
+gint i;
+Fixture *f = user_data;
+spice_file_transfer_task_flush_done(xfer_task, NULL);
+agent_send_error_async(xfer_task);
+}
+
+static void
+test_agent_error_on_read(Fixture *f, gconstpointer user_data)
+{
+GList *it;
+
+f->tasks = spice_file_transfer_task_create_tasks(NULL,
+ f->files,
+ G_FILE_COPY_NONE,
+ f->cancellable,
+ 
transfer_flush_callback_agent_error,
+ f,
+ transfer_done,
+ f);
+for (it = f->tasks; it != NULL; it = it->next) {
+SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(it->data);
+g_signal_connect(xfer_task, "file-info", 
G_CALLBACK(transfer_task_on_file_info), f);
+spice_file_transfer_task_start_task(xfer_task);
+}
+g_main_loop_run (f->loop);
+}
+
 /* Tests summary:
  *
  * This tests are specific to SpiceFileTransferTask and how it handles the
@@ -302,6 +376,14 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(SINGLE_FILE),
f_setup, test_cancel_on_read_file, f_teardown);
 
+g_test_add("/spice-file-transfer-task/single/agent/cancel",
+   Fixture, GUINT_TO_POINTER(SINGLE_FILE),
+   f_setup, test_agent_cancel_on_read, f_teardown);
+
+g_test_add("/spice-file-transfer-task/single/agent/error",
+   Fixture, GUINT_TO_POINTER(SINGLE_FILE),
+   f_setup, test_agent_error_on_read, f_teardown);
+
 g_test_add("/spice-file-transfer-task/multiple/simple-transfer",
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_simple_transfer, f_teardown);
@@ -318,5 +400,13 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_cancel_on_read_file, f_teardown);
 
+g_test_add("/spice-file-transfer-task/multiple/agent/cancel",
+   Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
+   f_setup, test_agent_cancel_on_read, f_teardown);
+
+g_test_add("/spice-file-transfer-task/multiple/agent/error",
+

[Spice-devel] [spice-gtk v3 14/16] tests: file-transfer cancel on file-info

2016-05-30 Thread Victor Toso 
---
 tests/file-transfer.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/tests/file-transfer.c b/tests/file-transfer.c
index 1421f1e..d8dcf6c 100644
--- a/tests/file-transfer.c
+++ b/tests/file-transfer.c
@@ -170,6 +170,47 @@ test_cancel_on_start_task(Fixture *f, gconstpointer 
user_data)
 g_main_loop_run (f->loop);
 }
 
+/***
+ * TEST CANCEL ON FILE-INFO
+ 
**/
+
+static void
+transfer_flush_callback_cancel_before_flush(SpiceFileTransferTask *xfer_task,
+void *buffer,
+gssize count,
+gpointer user_data)
+{
+gint i;
+Fixture *f = user_data;
+for (i = 0; i < f->num_files; i++) {
+g_test_expect_message("GSpice", G_LOG_LEVEL_CRITICAL, "*assertion 
'channel != NULL'*");
+}
+g_cancellable_cancel(f->cancellable);
+spice_file_transfer_task_flush_done(xfer_task, NULL);
+agent_send_success_async(xfer_task);
+}
+
+static void
+test_cancel_on_file_info(Fixture *f, gconstpointer user_data)
+{
+GList *it;
+
+f->tasks = spice_file_transfer_task_create_tasks(NULL,
+ f->files,
+ G_FILE_COPY_NONE,
+ f->cancellable,
+ 
transfer_flush_callback_cancel_before_flush,
+ f,
+ transfer_done,
+ f);
+for (it = f->tasks; it != NULL; it = it->next) {
+SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(it->data);
+g_signal_connect(xfer_task, "file-info", 
G_CALLBACK(transfer_task_on_file_info), f);
+spice_file_transfer_task_start_task(xfer_task);
+}
+g_main_loop_run (f->loop);
+}
+
 /* Tests summary:
  *
  * This tests are specific to SpiceFileTransferTask and how it handles the
@@ -212,6 +253,10 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(SINGLE_FILE),
f_setup, test_cancel_on_start_task, f_teardown);
 
+g_test_add("/spice-file-transfer-task/single/cancel/on-file-info",
+   Fixture, GUINT_TO_POINTER(SINGLE_FILE),
+   f_setup, test_cancel_on_file_info, f_teardown);
+
 g_test_add("/spice-file-transfer-task/multiple/simple-transfer",
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_simple_transfer, f_teardown);
@@ -220,5 +265,9 @@ int main(int argc, char* argv[])
Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
f_setup, test_cancel_on_start_task, f_teardown);
 
+g_test_add("/spice-file-transfer-task/multiple/cancel/on-file-info",
+   Fixture, GUINT_TO_POINTER(MULTIPLE_FILES),
+   f_setup, test_cancel_on_file_info, f_teardown);
+
 return g_test_run();
 }
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 10/16] tests: fix build with smartcard enabled

2016-05-30 Thread Victor Toso 
In file included from
../spice-common/common/client_marshallers.h:29:0,
from ../src/spice-channel-priv.h:35,
from ../src/spice-file-transfer-task-priv.h:28,
from file-transfer.c:3:
../spice-common/common/messages.h:45:23: fatal error: libcacard.h: No such file 
or directory
compilation terminated.
---
 tests/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c1d95c1..d56ff1b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,6 +20,7 @@ noinst_PROGRAMS += $(TESTS)
 AM_CPPFLAGS =  \
$(COMMON_CFLAGS)\
$(GIO_CFLAGS)   \
+   $(SMARTCARD_CFLAGS) \
-I$(top_srcdir)/src \
-I$(top_builddir)/src   \
-DG_LOG_DOMAIN=\"GSpice\"   \
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 01/16] file-xfer: get functions for SpiceFileTransferTask

2016-05-30 Thread Victor Toso 
In order to channel-main interact with each SpiceFileTransferTask for
the file-transfer operation, the following functions are introduced:
* spice_file_transfer_task_get_id
* spice_file_transfer_task_get_channel
* spice_file_transfer_task_get_cancellable

Note that although "id" property is public and could be acquired by
g_object_get but having the helper function is more practical.

This change is related to split SpiceFileTransferTask from
channel-main.
---
 src/channel-main.c | 32 +---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 2f29312..89675d5 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -54,6 +54,10 @@
 
 typedef struct spice_migrate spice_migrate;
 
+static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
+static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
+static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
+
 /**
  * SECTION:file-transfer-task
  * @short_description: Monitoring file transfers
@@ -1902,9 +1906,12 @@ static void file_xfer_data_flushed_cb(GObject 
*source_object,
 static void file_xfer_queue(SpiceFileTransferTask *self, int data_size)
 {
 VDAgentFileXferDataMessage msg;
-SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(self->channel);
+SpiceMainChannel *channel;
 
-msg.id = self->id;
+channel = spice_file_transfer_task_get_channel(self);
+g_return_if_fail(channel != NULL);
+
+msg.id = spice_file_transfer_task_get_id(self);
 msg.size = data_size;
 agent_msg_queue_many(channel, VD_AGENT_FILE_XFER_DATA,
  , sizeof(msg),
@@ -1932,12 +1939,15 @@ static void file_xfer_read_cb(GObject *source_object,
 }
 
 if (count > 0 || self->file_size == 0) {
+GCancellable *cancellable;
+
 self->read_bytes += count;
 g_object_notify(G_OBJECT(self), "progress");
 file_xfer_queue(self, count);
 if (count == 0)
 return;
-file_xfer_flush_async(channel, self->cancellable,
+cancellable = spice_file_transfer_task_get_cancellable(self);
+file_xfer_flush_async(channel, cancellable,
   file_xfer_data_flushed_cb, self);
 self->pending = TRUE;
 } else if (error) {
@@ -3221,7 +3231,23 @@ gboolean spice_main_file_copy_finish(SpiceMainChannel 
*channel,
 return g_task_propagate_boolean(task, error);
 }
 
+static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self)
+{
+g_return_val_if_fail(self != NULL, 0);
+return self->id;
+}
+
+static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self)
+{
+g_return_val_if_fail(self != NULL, NULL);
+return self->channel;
+}
 
+static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self)
+{
+g_return_val_if_fail(self != NULL, NULL);
+return self->cancellable;
+}
 
 static void
 spice_file_transfer_task_get_property(GObject *object,
-- 
2.5.5

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

[Spice-devel] [spice-gtk v3 02/16] file-xfer: introduce flush_callback and flush_done

2016-05-30 Thread Victor Toso 
By introducing a flush_callback such as SpiceFileTransferTaskFlushCb
SpiceFileTransferTask becomes agnostic on how channel-main flushes
the data.

The spice_file_transfer_task_flush_done() function is now introduced
to tell SpiceFileTransferTask that flushing is over and we can read
more data if no error has happened.

This change is related to split SpiceFileTransferTask from
channel-main.
---
 src/channel-main.c | 143 -
 1 file changed, 88 insertions(+), 55 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 89675d5..702f146 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -54,9 +54,14 @@
 
 typedef struct spice_migrate spice_migrate;
 
+typedef void (*SpiceFileTransferTaskFlushCb)(SpiceFileTransferTask *xfer_task,
+ void *buffer,
+ gssize count,
+ gpointer user_data);
 static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
 static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
 static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
+static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self, 
GError *error);
 
 /**
  * SECTION:file-transfer-task
@@ -89,6 +94,8 @@ struct _SpiceFileTransferTask
 GCancellable   *cancellable;
 GFileProgressCallback  progress_callback;
 gpointer   progress_callback_data;
+SpiceFileTransferTaskFlushCb   flush_callback;
+gpointer   flush_callback_data;
 GAsyncReadyCallbackcallback;
 gpointer   user_data;
 char   *buffer;
@@ -1859,73 +1866,49 @@ static void file_xfer_data_flushed_cb(GObject 
*source_object,
 SpiceMainChannel *channel = (SpiceMainChannel *)source_object;
 GError *error = NULL;
 
-self->pending = FALSE;
 file_xfer_flush_finish(channel, res, );
-if (error || self->error) {
-spice_file_transfer_task_completed(self, error);
-return;
-}
-
-if (spice_util_get_debug()) {
-const GTimeSpan interval = 20 * G_TIME_SPAN_SECOND;
-gint64 now = g_get_monotonic_time();
-
-if (interval < now - self->last_update) {
-gchar *basename = g_file_get_basename(self->file);
-self->last_update = now;
-SPICE_DEBUG("transferred %.2f%% of the file %s",
-100.0 * self->read_bytes / self->file_size, basename);
-g_free(basename);
-}
-}
-
-if (self->progress_callback) {
-goffset read = 0;
-goffset total = 0;
-SpiceMainChannel *main_channel = self->channel;
-GHashTableIter iter;
-gpointer key, value;
-
-/* since the progress_callback does not have a parameter to indicate
- * which file the progress is associated with, report progress on all
- * current transfers */
-g_hash_table_iter_init(, main_channel->priv->file_xfer_tasks);
-while (g_hash_table_iter_next(, , )) {
-SpiceFileTransferTask *t = (SpiceFileTransferTask *)value;
-read += t->read_bytes;
-total += t->file_size;
-}
-
-self->progress_callback(read, total, self->progress_callback_data);
-}
-
-/* Read more data */
-file_xfer_continue_read(self);
+spice_file_transfer_task_flush_done(self, error);
 }
 
-static void file_xfer_queue(SpiceFileTransferTask *self, int data_size)
+static void file_xfer_queue(SpiceFileTransferTask *xfer_task, void *buffer, 
int data_size)
 {
 VDAgentFileXferDataMessage msg;
 SpiceMainChannel *channel;
 
-channel = spice_file_transfer_task_get_channel(self);
+channel = spice_file_transfer_task_get_channel(xfer_task);
 g_return_if_fail(channel != NULL);
 
-msg.id = spice_file_transfer_task_get_id(self);
+msg.id = spice_file_transfer_task_get_id(xfer_task);
 msg.size = data_size;
 agent_msg_queue_many(channel, VD_AGENT_FILE_XFER_DATA,
  , sizeof(msg),
- self->buffer, data_size, NULL);
+ buffer, data_size, NULL);
 spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
 }
 
+static void file_xfer_flush_callback(SpiceFileTransferTask *xfer_task,
+ void *buffer,
+ gssize count,
+ gpointer user_data)
+{
+SpiceMainChannel *main_channel;
+GCancellable *cancellable;
+
+file_xfer_queue(xfer_task, buffer, count);
+if (count == 0)
+return;
+
+main_channel = spice_file_transfer_task_get_channel(xfer_task);
+cancellable = spice_file_transfer_task_get_cancellable(xfer_task);
+

[Spice-devel] [spice-gtk v3 03/16] file-xfer: introduce create_tasks and start_task

2016-05-30 Thread Victor Toso 
By splitting file_xfer_send_start_msg_async we can separate in three
different steps the spice_main_file_copy_async function:

1-) Creating tasks with spice_file_transfer_task_create_tasks which
now returns a GList of SpiceFileTransferTask;
2-) Setting handlers before the SpiceFileTransferTask starts;
3-) Starting the task with spice_file_transfer_task_start_task

(1) and (3) can be done outside channel-main scope.

This change is related to split SpiceFileTransferTask from
channel-main.
---
 src/channel-main.c | 150 +++--
 1 file changed, 88 insertions(+), 62 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 702f146..7843aeb 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -62,6 +62,17 @@ static guint32 
spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
 static SpiceMainChannel 
*spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
 static GCancellable 
*spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
 static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self, 
GError *error);
+static GList *spice_file_transfer_task_create_tasks(SpiceMainChannel *channel,
+GFile **files,
+GFileCopyFlags flags,
+GCancellable *cancellable,
+GFileProgressCallback 
progress_callback,
+gpointer 
progress_callback_data,
+
SpiceFileTransferTaskFlushCb flush_callback,
+gpointer 
flush_callback_data,
+GAsyncReadyCallback 
callback,
+gpointer user_data);
+static void spice_file_transfer_task_start_task(SpiceFileTransferTask *self);
 
 /**
  * SECTION:file-transfer-task
@@ -3066,59 +3077,9 @@ static void task_finished(SpiceFileTransferTask *task,
   gpointer data)
 {
 SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(data);
-g_hash_table_remove(channel->priv->file_xfer_tasks, 
GUINT_TO_POINTER(task->id));
-}
-
-static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
-   GFile **files,
-   GFileCopyFlags flags,
-   GCancellable *cancellable,
-   GFileProgressCallback 
progress_callback,
-   gpointer progress_callback_data,
-   SpiceFileTransferTaskFlushCb 
flush_callback,
-   gpointer flush_callback_data,
-   GAsyncReadyCallback callback,
-   gpointer user_data)
-{
-SpiceMainChannelPrivate *c = channel->priv;
-SpiceFileTransferTask *task;
-gint i;
-
-for (i = 0; files[i] != NULL && !g_cancellable_is_cancelled(cancellable); 
i++) {
-GCancellable *task_cancellable = cancellable;
-/* if a cancellable object was not provided for the overall operation,
- * create a separate object for each file so that they can be cancelled
- * separately  */
-if (!task_cancellable)
-task_cancellable = g_cancellable_new();
+guint32 task_id = spice_file_transfer_task_get_id(task);
 
-task = spice_file_transfer_task_new(channel, files[i], 
task_cancellable);
-task->flags = flags;
-task->progress_callback = progress_callback;
-task->progress_callback_data = progress_callback_data;
-task->flush_callback = flush_callback;
-task->flush_callback_data = flush_callback_data;
-task->callback = callback;
-task->user_data = user_data;
-
-CHANNEL_DEBUG(channel, "Insert a xfer task:%d to task list", task->id);
-g_hash_table_insert(c->file_xfer_tasks,
-GUINT_TO_POINTER(task->id),
-task);
-g_signal_connect(task, "finished", G_CALLBACK(task_finished), channel);
-g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0, task);
-
-g_file_read_async(files[i],
-  G_PRIORITY_DEFAULT,
-  cancellable,
-  file_xfer_read_async_cb,
-  g_object_ref(task));
-task->pending = TRUE;
-
-/* if we created a per-task cancellable above, free it */
-if (!cancellable)
-g_object_unref(task_cancellable);
-}
+g_hash_table_remove(channel->priv->file_xfer_tasks, 
GUINT_TO_POINTER(task_id));
 }
 
 /**
@@ -3164,6 +3125,7 @@