Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

2018-02-15 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 10:43:26PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 17:29, Christophe Fergeau wrote: > > > > This changes the suggested style to what is currently used in > > spice-server codebase. This also removes a few sent

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

2018-02-15 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 11:24:50PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 14:35, Christophe Fergeau wrote: > > > > This one sounds more like an RFC to me > > Well, this is really a bug fix in the documentation more than a RFC. > >

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

2018-02-15 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 17:34, Christophe Fergeau wrote: > > > > On Wed, Feb 14, 2018 at 10:45:56AM -0500, Frediano Ziglio wrote: > >>> > >>> Shouldn't this go w

Re: [Spice-devel] [PATCH v3 10/11] Add guidelines about warnings and whitespaces

2018-02-15 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 10:53:04PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 14:37, Christophe Fergeau wrote: > > > > On Thu, Feb 08, 2018 at 12:25:30PM +0100, Christophe de Dinechin wrote: > >> From: Christophe de Dinechin > >>

Re: [Spice-devel] [PATCH v2 12/13] Add guidelines about warnings and whitespaces

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 11:09:35AM +0100, Victor Toso wrote: > Hi, > > On Thu, Feb 08, 2018 at 05:01:05AM -0500, Frediano Ziglio wrote: > > > > Depends on many cases. You don't want spurious changes to make harder to > > > > look at the history for instance (that is a point for Nack). > > > > The

Re: [Spice-devel] [PATCH spice-streaming-agent v3 3/3] mjpeg-fallback: unittest for the options parsing

2018-02-14 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 10:40:58AM -0500, Frediano Ziglio wrote: > > > > > On 14 Feb 2018, at 13:34, Lukáš Hrázký wrote: > > > > > > Introduce a unit test framework (Catch) to the codebase and a simple > > > unit test for parsing the options of the mjpeg plugin. > > > > > > Signed-off-by: Lukáš

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

2018-02-14 Thread Christophe Fergeau
On Wed, Feb 14, 2018 at 10:45:56AM -0500, Frediano Ziglio wrote: > > > > Shouldn't this go with a Makefile rule? A few lines in the log what this > > is about, what is the goal for having this file, ... would not hurt. > > > > Christophe > > > > I think this file is supposed to just help develo

[Spice-devel] [spice-server] style: Slight tweak to the header guard section

2018-02-14 Thread Christophe Fergeau
This changes the suggested style to what is currently used in spice-server codebase. This also removes a few sentences which are not really related to how one should format their header guards. Signed-off-by: Christophe Fergeau --- docs/spice_style.txt | 6 ++ 1 file changed, 2 insertions

Re: [Spice-devel] [PATCH v3 09/11] Add mention of header guards

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 12:25:29PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > docs/spice_style.txt | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/docs/spice_style.txt b/docs/spice_style.tx

Re: [Spice-devel] [PATCH spice-streaming-agent v2 3/4] use std::string where it's easy to replace

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 08:16:37AM +0100, Christophe de Dinechin wrote: > Using std::string by default is *not* considered a good practice in > C++. The reference for C++ good practices are probably best summarized > by the C++ Core Guidelines: > https://github.com/isocpp/CppCoreGuidelines/blob/mas

Re: [Spice-devel] [PATCH spice-streaming-agent] build: Do not use top_srcdir if not necessary

2018-02-14 Thread Christophe Fergeau
On Mon, Jan 29, 2018 at 05:16:12PM -0500, Frediano Ziglio wrote: > > > > > > > On 29 Jan 2018, at 12:00, Frediano Ziglio wrote: > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > > Makefile.am | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/Makefi

Re: [Spice-devel] [PATCH v3 10/11] Add guidelines about warnings and whitespaces

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 12:25:30PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > The objective of these guidelines is that: > - We avoid introducing new warnings > - We know how to fix old ones > - We don't have to isolate whitespace changes when submitting patches, >

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

2018-02-14 Thread Christophe Fergeau
This one sounds more like an RFC to me, as from a quick look in server/, this is not the style currently in use. Christophe On Thu, Feb 08, 2018 at 12:25:31PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > As written, the headers style guide looks quite wrong. In partic

Re: [Spice-devel] [PATCH v3 03/11] Specify file extensions for C++ and Obj-C

2018-02-14 Thread Christophe Fergeau
On Mon, Feb 12, 2018 at 03:23:06AM -0500, Frediano Ziglio wrote: > See > https://lists.freedesktop.org/archives/spice-devel/2018-February/041724.html > > > From: Christophe de Dinechin > > > > Signed-off-by: Christophe de Dinechin > > --- > > docs/spice_style.txt | 11 ++- > > 1 file

Re: [Spice-devel] [PATCH v2 11/13] Add mention of header guards

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 03:58:46AM -0500, Frediano Ziglio wrote: > > + > > +[source,h] > > +--- > > +#ifndef MY_MODULE_H > > +#define MY_MODULE_H > > + > > +... > > + > > +#endif /* MY_MODULE_H */ > > +--- > > + > > Are we suggesting C style only comment or is clear the is not important? > (I'm ju

Re: [Spice-devel] [PATCH v2 06/13] Rephrase section on short functions for readability

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 03:50:37AM -0500, Frediano Ziglio wrote: > > > > From: Christophe de Dinechin > > > > Signed-off-by: Christophe de Dinechin > > --- > > docs/spice_style.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/docs/spice_style.txt b/docs/spice

Re: [Spice-devel] [PATCH v2 05/13] Rephrase section about constants

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 03:48:46AM -0500, Frediano Ziglio wrote: > I think that the "Alternatively, use global `const` variables." > is quite an ancient comment when spice-server was in C++ > (much before I join and I think even older than the git repository!) There was a C++ *client* in spice.git

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

2018-02-14 Thread Christophe Fergeau
Shouldn't this go with a Makefile rule? A few lines in the log what this is about, what is the goal for having this file, ... would not hurt. Christophe On Thu, Feb 08, 2018 at 12:25:21PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinech

Re: [Spice-devel] [PATCH v3 04/11] Rephrase assertion section

2018-02-14 Thread Christophe Fergeau
On Thu, Feb 08, 2018 at 12:25:24PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > docs/spice_style.txt | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/docs/spice_style.txt b/docs/sp

Re: [Spice-devel] [PATCH v3 04/11] Rephrase assertion section

2018-02-14 Thread Christophe Fergeau
On Tue, Feb 13, 2018 at 02:40:38PM +0100, Christophe de Dinechin wrote: > > > > On 13 Feb 2018, at 14:39, Frediano Ziglio wrote: > > > >>> > >>> On 12 Feb 2018, at 17:58, Frediano Ziglio wrote: > >>> > >>> In SPICE, as in Glib, ERROR is more "strong" than CRITICAL > >>> (I don't know where t

Re: [Spice-devel] [PATCH spice-common 5/8] canvas: Remove dc parameter from SwCanvas::put_image

2018-01-18 Thread Christophe Fergeau
On Wed, Jan 17, 2018 at 11:31:07AM +, Frediano Ziglio wrote: > Not used anymore. > > Signed-off-by: Frediano Ziglio > --- > common/canvas_base.h | 3 --- > common/sw_canvas.c | 3 --- > 2 files changed, 6 deletions(-) > > diff --git a/common/canvas_base.h b/common/canvas_base.h > index 2d

Re: [Spice-devel] [PATCH spice-common 2/8] canvas: Remove mutex field from PixmanData

2018-01-18 Thread Christophe Fergeau
On Wed, Jan 17, 2018 at 11:31:04AM +, Frediano Ziglio wrote: > The field is only assigned but never used. I would mention that this was used in the GDI canvas, but that this has now been removed. Apart from this, for the series, Acked-by: Christophe Fergeau > > Signed-off-by:

Re: [Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

2018-01-18 Thread Christophe Fergeau
On Thu, Jan 18, 2018 at 12:06:34PM +0100, Marc-André Lureau wrote: > Hi > > On Thu, Jan 18, 2018 at 10:31 AM, Christophe Fergeau > wrote: > > At least on X.org, malicious code could run the equivalent of "watch > > xsel -o --clipboard" in a VM, and w

[Spice-devel] [spice-gtk] Add --spice-disable-clipboard option

2018-01-18 Thread Christophe Fergeau
At least on X.org, malicious code could run the equivalent of "watch xsel -o --clipboard" in a VM, and would then be able to track all the clipboard content, even when the spice-gtk widget is not focused. At the moment, applications call spice_set_session_option(), and then set SpiceGtkSession::au

Re: [Spice-devel] xf86-video-qxl + libspice-server: no image on FreeBSD

2018-01-17 Thread Christophe Fergeau
hey, On Wed, Jan 17, 2018 at 12:15:03PM +0300, Oleg Ginzburg wrote: > Confirm - these changes work successfully, tested on: FreeBSD 11.1-RELEASE > and FreeBSD 12-CURRENT (aka HEAD). > > Will these changes be included in the next release? > > There is another small issue of build libspice-server

Re: [Spice-devel] [PATCH spice-server] tests: Remove test-just-sockets-no-ssl

2018-01-16 Thread Christophe Fergeau
ould have argued that test-display-base is more centered around testing display-related stuff, while this one could focus on testing socket-related things. But since it's not part of automated tests, ok for removing it. Acked-by: Christophe Fergeau signature.asc Description:

Re: [Spice-devel] [PATCH spice-server] red-stream: Remove AsyncRead::stream

2018-01-16 Thread Christophe Fergeau
On Tue, Jan 16, 2018 at 08:59:14AM -0500, Frediano Ziglio wrote: > Yes, is handling a particular case (reading 0 bytes) directly instead of > adding a pending operation. It can be useful if you have a sequence > length+data > where length can be 0. Currently cannot happen (size is always > 0), in

Re: [Spice-devel] [PATCH spice-server] red-stream: Remove AsyncRead::stream

2018-01-16 Thread Christophe Fergeau
read_done_cb(opaque); > +return; > +} This bit seems unrelated? Apart from this, Acked-by: Christophe Fergeau Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] red-stream: Reuse red_stream_disable_writev function

2018-01-16 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Tue, Jan 09, 2018 at 07:43:25PM +, Frediano Ziglio wrote: > The same function is used to reset writev field in SASL code. > > Signed-off-by: Frediano Ziglio > --- > server/red-stream.c | 4 +--- > 1 file changed, 1 insertion(+), 3 dele

Re: [Spice-devel] [PATCH spice-server] basic-event-loop: Document why code does not use default GLib context

2018-01-16 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Tue, Jan 09, 2018 at 07:43:24PM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/tests/basic-event-loop.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/server/tests/basic-event-loop.c b/server/tests/

Re: [Spice-devel] [PATCH spice-server 01/11] stat-file: Protect flags field with atomic operation

2018-01-16 Thread Christophe Fergeau
On Tue, Jan 09, 2018 at 04:42:51PM -0500, Frediano Ziglio wrote: > > > > On Mon, Dec 11, 2017 at 10:27:58AM +, Frediano Ziglio wrote: > > > Coverity complaint that this field should be protected by > > > a mutex as other accesses are with the mutex locked. > > > Use atomic operation. Not in an

Re: [Spice-devel] [PATCH spice-server] Minor compatibility with FreeBSD system

2018-01-15 Thread Christophe Fergeau
Hey, Might have been worth splitting this a bit. Regardless of that, looks goodh to me, Acked-by: Christophe Fergeau On Wed, Jan 10, 2018 at 08:51:16AM +, Frediano Ziglio wrote: > Some additional header are needed to avoid undefined types. > SOL_TCP and IPPROTO_TCP have the same va

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-12 Thread Christophe Fergeau
On Fri, Jan 12, 2018 at 08:05:24AM -0500, Marc-André Lureau wrote: > > Is it really a security reason the clipboard behaviour is different on > Wayland? I don't know the reason for this behaviour, for me this is similar to preventing applications from capturing the whole screen. https://wiki.x.or

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-12 Thread Christophe Fergeau
On Thu, Jan 11, 2018 at 12:35:36PM -0500, Marc-André Lureau wrote: > > I agree with you that some help from the windowing/toolkit would be good > > to have, but in this case, I doubt we are going to be able to do better > > than managing this in spice-gtk. > > Yet it is already being solved at a l

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-11 Thread Christophe Fergeau
On Thu, Jan 11, 2018 at 11:42:00AM -0500, Marc-André Lureau wrote: > > > (it seems that clipboard system for GTK 4 has been reworked quite a > > bit - see https://git.gnome.org/browse/gtk+/log/?h=wip/otte/clipboard > > - this is already merged into master) > > I have not much time to look at the

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-11 Thread Christophe Fergeau
On Wed, Jan 10, 2018 at 06:48:14PM -0500, Marc-André Lureau wrote: > Hi > > - Original Message - > > On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote: > > > I think it's problematic for traditional applications as well. > > > clipboard access is probably going to be limite

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-10 Thread Christophe Fergeau
On Tue, Jan 09, 2018 at 12:16:33PM -0500, Marc-André Lureau wrote: > I think it's problematic for traditional applications as well. > clipboard access is probably going to be limited by default and only > accessed through so-called "portals", just like file access etc. This > topic should be brough

Re: [Spice-devel] [RFC PATCH 0/3] Flush interface and TCP_CORK

2018-01-10 Thread Christophe Fergeau
On Wed, Jan 10, 2018 at 11:39:32AM -0500, Frediano Ziglio wrote: > > > > Hey, > > > > On Fri, Dec 15, 2017 at 04:33:26AM -0500, Frediano Ziglio wrote: > > > ping > > > > So I finally took a quick look at this, patch 2/3 and 3/3 really deserve > > a more detailed commit log/rationale/..., they ca

Re: [Spice-devel] [PATCH spice-server v2 2/2] stream-device: Implements properly device reset on open/close

2018-01-10 Thread Christophe Fergeau
On Thu, Dec 07, 2017 at 08:47:39AM +, Frediano Ziglio wrote: > Due to the way Qemu handle the device we must consume all pending > data inside the callback to read data from the device. Any idea if QEMU can be fixed to do the right thing? We'll still need some kind of workaround to deal with o

Re: [Spice-devel] [PATCH spice-server v2 1/2] test-stream-device: Better Qemu emulation for data reading

2018-01-10 Thread Christophe Fergeau
On Thu, Dec 07, 2017 at 08:47:38AM +, Frediano Ziglio wrote: > Qemu does not trigger a new data read if we don't read all data in > the buffer. > > Signed-off-by: Frediano Ziglio > --- > server/stream-device.c| 9 > server/tests/test-stream-device.c | 43 >

Re: [Spice-devel] [RFC PATCH 0/3] Flush interface and TCP_CORK

2018-01-10 Thread Christophe Fergeau
Hey, On Fri, Dec 15, 2017 at 04:33:26AM -0500, Frediano Ziglio wrote: > ping So I finally took a quick look at this, patch 2/3 and 3/3 really deserve a more detailed commit log/rationale/..., they cannot have just a shortlog. Apart from that, they seemed ok, though we don't really have a way to c

Re: [Spice-devel] [PATCH spice-server v6 00/10] SASL code improvements

2018-01-09 Thread Christophe Fergeau
improved if needed. For the series, Acked-by: Christophe Fergeau On Tue, Jan 09, 2018 at 07:44:55AM +, Frediano Ziglio wrote: > SASL authentication code were a bit dodgy to see. > The series have 2 parts: > - a test for SASL code; > - some code refactory. > > Test is not th

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-09 Thread Christophe Fergeau
On Tue, Jan 09, 2018 at 11:08:44AM -0500, Marc-André Lureau wrote: > - Original Message - > > > > What makes spice different here is that it's used to access a VM, and a > > VM is supposed to give you isolation. If some hostile code is running in > > the VM, its impact on the host/client O

Re: [Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-09 Thread Christophe Fergeau
Hey, On Tue, Jan 09, 2018 at 09:46:48AM -0500, Marc-André Lureau wrote: > - Original Message - > > This is used to prevent unfocused guests from sniffing the clipboard > > content without the host or other guests noticing. This can be a > > security issue if any VM can track the clipboard

Re: [Spice-devel] [PATCH spice-server] test-channel: Use correct endianness for ack message

2018-01-09 Thread Christophe Fergeau
For the series: Acked-by: Christophe Fergeau On Fri, Jan 05, 2018 at 12:12:00PM +, Frediano Ziglio wrote: > Network fields should be encoded as little endian. > This was discovered using an emulated MIPS machine. > > Signed-off-by: Frediano Ziglio > --- > server/tests/

[Spice-devel] RFC [spice-gtk] session: Allow to delay sending clipboard to the guest

2018-01-09 Thread Christophe Fergeau
est sending of clipboard data is then delayed until the window is focused again. This behaviour matches the behaviour we get on Wayland. This mostly solves https://bugzilla.redhat.com/show_bug.cgi?id=1320263 Signed-off-by: Christophe Fergeau --- Not overly happy with the added complexity, ma

Re: [Spice-devel] How to configure spice proxy with virt-viewer

2018-01-08 Thread Christophe Fergeau
On Mon, Jan 08, 2018 at 04:05:30PM +0800, 王杰东 wrote: > When my virt-viewer spice clients are in net-A , and the hypervisor server > with spice is in net-B , and a server is in both net-A and net-B > How can i use virt-viewer to connect th vms on hypervisor server ? > > https://access.redhat.com/

Re: [Spice-devel] [PATCH spice-gtk 1/2] gstreamer: use custom playbin sink

2018-01-08 Thread Christophe Fergeau
On Mon, Jan 08, 2018 at 04:47:26AM -0500, Frediano Ziglio wrote: > > > > Use custom playbin sink instead of just appsink. > > In order to allow playbin to choose decoders that requires > > more complex pipelines (e.g. pipeline that requires color > > space conversion & uses gl memory). > > The new

[Spice-devel] [spice-gtk 2/2] build-sys: Workaround missing openssl.pc for FreeBSD

2018-01-08 Thread Christophe Fergeau
From: Ting-Wei Lan FreeBSD has OpenSSL installed in base, but .pc files are not available. We can still successfully build spice-gtk by setting SSL_CFLAGS and SSL_LIBS variables manually, but openssl will still be recorded to Requires.private field of spice-client-glib-2.0.pc, causing problems fo

[Spice-devel] [spice-gtk 0/2] FreeBSD build fixes

2018-01-08 Thread Christophe Fergeau
Hey, Here are 2 build fixes for FreeBSD which were filed in bugzilla ([1] and [2]). They look good to me, so I'll push them in a few days if nobody objects in the mean time. Christophe [1] https://bugs.freedesktop.org/show_bug.cgi?id=104524 [2] https://bugs.freedesktop.org/show_bug.cgi?id=104525

[Spice-devel] [spice-gtk 1/2] build-sys: Enable ACL support on FreeBSD

2018-01-08 Thread Christophe Fergeau
From: Ting-Wei Lan sys/acl.h works on both FreeBSD and Linux, so we should use it instead of acl/libacl.h, which only works on Linux. FreeBSD puts ACL functions in libc, so we should check whether ACL functions are available in libc before doing the check with -lacl. https://bugs.freedesktop.or

Re: [Spice-devel] [PATCH spice-server v4 6/9] Handle SASL initialisation mainly in red-stream.c

2018-01-05 Thread Christophe Fergeau
On Fri, Jan 05, 2018 at 03:45:31PM +, Frediano Ziglio wrote: > -static void reds_handle_auth_sasl_start(void *opaque) > +static void reds_handle_sasl_result(void *opaque, RedSaslError status) > { > RedLinkInfo *link = (RedLinkInfo *)opaque; > -RedSaslError status; > - > -status =

Re: [Spice-devel] [PATCH spice-server v3 11/12] red-stream: Unify start and step passes

2018-01-05 Thread Christophe Fergeau
which is only needed for _start, and can be NULL afterwards. Apart from this, looks good, Acked-by: Christophe Fergeau signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.or

Re: [Spice-devel] [PATCH spice-server v3 12/12] red-stream: Encapsulate all authentication state in RedSASLAuth

2018-01-05 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:13AM +, Frediano Ziglio wrote: > Instead of having half state in RedSASL and half in RedSASLAuth > move everything in RedSASLAuth. This also reduces memory usage > when we are using SASL but we finish the authentication step. > > Signed-off-by: Frediano Ziglio >

Re: [Spice-devel] [PATCH spice-server v3 10/12] red-stream: Handle properly endianness in SASL code

2018-01-05 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:11AM +, Frediano Ziglio wrote: > All SPICE protocol is little endian, there's no agreement on other > endian and currently we support only little endian so make sure > this will work even possibly running on a big endian machine. > > Signed-off-by: Frediano Ziglio

Re: [Spice-devel] [PATCH spice-server v3 09/12] Handle SASL initialisation mainly in red-stream.c

2018-01-05 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:10AM +, Frediano Ziglio wrote: > Asynchronous code jumping from a file to another is tedious to read > also having code handling the same stuff in two files does not look > a good design. > > Signed-off-by: Frediano Ziglio > --- > server/red-stream.c | 118 > ++

Re: [Spice-devel] [PATCH spice-server v3 07/12] test-sasl: Add tests for different mechanism names

2018-01-05 Thread Christophe Fergeau
On Fri, Jan 05, 2018 at 04:47:55AM -0500, Frediano Ziglio wrote: > > > > On Fri, Dec 22, 2017 at 10:07:08AM +, Frediano Ziglio wrote: > > > @@ -395,16 +427,32 @@ idle_next_test(void *arg) > > > { > > > // end previous test > > > if (test_num >= 0) { > > > -g_assert(encode_ca

Re: [Spice-devel] [PATCH spice-server v3 07/12] test-sasl: Add tests for different mechanism names

2018-01-04 Thread Christophe Fergeau
Ok, Acked-by: Christophe Fergeau On Fri, Dec 22, 2017 at 10:07:08AM +, Frediano Ziglio wrote: > Try different connections with different tricky names. > > Signed-off-by: Frediano Ziglio > --- > server/tests/test-sasl.c | 60 >

Re: [Spice-devel] [PATCH spice-server v3 06/12] test-sasl: Base test, connect using SASL

2018-01-04 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:07AM +, Frediano Ziglio wrote: > Create a thread that emulate a client and start SASL authentication ".. that emulates .." > > Signed-off-by: Frediano Ziglio > --- > server/tests/test-sasl.c | 236 > ++- > 1 file ch

Re: [Spice-devel] [PATCH spice-server v3 07/12] test-sasl: Add tests for different mechanism names

2018-01-04 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:08AM +, Frediano Ziglio wrote: > @@ -395,16 +427,32 @@ idle_next_test(void *arg) > { > // end previous test > if (test_num >= 0) { > -g_assert(encode_called); > +const TestData *data = &tests_data[test_num]; > +if (data->success)

Re: [Spice-devel] [PATCH spice-server v3 05/12] test-sasl: Add code for mocking function to test state

2018-01-04 Thread Christophe Fergeau
In the shortlog, "Add code *to* mocking functions .."? On Fri, Dec 22, 2017 at 10:07:06AM +, Frediano Ziglio wrote: > Check some functions are called in a given sequence. > > Signed-off-by: Frediano Ziglio > --- > server/tests/test-sasl.c | 19 +++ > 1 file changed, 19 inser

Re: [Spice-devel] [PATCH spice-server v3 04/12] test-sasl: Initial SASL test

2018-01-04 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:05AM +, Frediano Ziglio wrote: > +int > +main(int argc, char *argv[]) > +{ > +return 0; > +} > +#else > +int > +main(void) > +{ > +return 0; > +} > +#endif Have you considered disabling the test in Makefile.am when SASL is not available? Christophe sign

Re: [Spice-devel] [PATCH spice-server v3 02/12] red-stream: Avoid infinite loop on sasl_encode/decode failure

2018-01-04 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:07:03AM +, Frediano Ziglio wrote: > These functions do not set errno so is possible that errno is EAGAIN. I would be a little bit more explicit: "errno is EAGAIN" -> "errno has a stale value which happens to be EAGAIN" Acked-by: Christo

[Spice-devel] Windows Guest Tools 0.141

2018-01-04 Thread Christophe Fergeau
Hi, A new release of the SPICE Guest Tools for Windows is available at https://www.spice-space.org/download/windows/spice-guest-tools/spice-guest-tools-0.141/spice-guest-tools-0.141.exe The SPICE Guest Tools for Windows is an all-in-one installer which will install the virtio drivers, the QXL dri

[Spice-devel] [spice-gtk 2/2] Implement GObject::constructed rather than ::constructor

2018-01-04 Thread Christophe Fergeau
A few classes are implementing GObject::constructor, which is more cumbersome to use than ::constructed. Other classes use ::constructed rather than ::constructor. This commit removes the last uses of ::constructor. Signed-off-by: Christophe Fergeau --- src/spice-gtk-session.c | 20

[Spice-devel] [spice-gtk 1/2] session: Remove unused spice_gtk_session_get_read_only prototype

2018-01-04 Thread Christophe Fergeau
There is no such function implemented anywhere in the codebase. Signed-off-by: Christophe Fergeau --- src/spice-gtk-session-priv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-priv.h index d7fe3133..0dbc9e96 100644 --- a/src/spice-gtk

Re: [Spice-devel] [PATCH spice-gtk 1/2] gstreamer: use custom playbin sink

2018-01-04 Thread Christophe Fergeau
On Sun, Dec 31, 2017 at 05:46:24PM +0200, Snir Sheriber wrote: > Use custom playbin sink instead of just appsink. > In order to allow playbin to choose decoders that requires > more complex pipelines (e.g. pipeline that requires color > space conversion & uses gl memory). What is gstreamer take on

Re: [Spice-devel] [NSIS] Do not install .pdb debug files on the target system

2018-01-02 Thread Christophe Fergeau
Ok, this is an alternative to the patch I sent a while ago, which keeps the files on the ISO, but does not add them to the installer. I was initially confused ;) Acked-by: Christophe Fergeau On Mon, Jan 01, 2018 at 10:15:19AM +0200, Yedidyah Bar David wrote: > Usually they are not needed,

Re: [Spice-devel] [PATCH spice-server v3 03/12] red-stream: Avoid to specify 2 mech names during SASL

2018-01-02 Thread Christophe Fergeau
On Tue, Jan 02, 2018 at 06:50:27AM -0500, Frediano Ziglio wrote: > > Acked-by: Snir Sheriber > > ( Maybe can be merged with the first patch? ) > > It touches the same lines but it a completely different issue and > rationale. To be fair, this patch does not really have a detailed issue/rationale

Re: [Spice-devel] [PATCH spice-common v2] canvas-base: Fix width computation for palette images

2017-12-22 Thread Christophe Fergeau
On Fri, Dec 22, 2017 at 10:02:37AM +, Frediano Ziglio wrote: > Palette images are encoded using a slightly larger number of pixel than > width. Hmm, these extra pixels are just padding, aren't they? "Palette images have padding at the end of each line, so their stride can't be inferred from th

Re: [Spice-devel] [PATCH spice-common] canvas-base: Fix width computation for palette images

2017-12-21 Thread Christophe Fergeau
upgrading from spice-gtk v0.31 to spice-gtk v0.33" I'd mention some of these in the commit log, but apart from this, Acked-by: Christophe Fergeau Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-dev

Re: [Spice-devel] [PATCH spice-server v2 03/13] red-stream: Avoid useless copy of mechlist

2017-12-20 Thread Christophe Fergeau
On Wed, Dec 20, 2017 at 09:35:00AM +, Frediano Ziglio wrote: > The list will persist while the SASL connection is not disposed > (or another sasl_listmech called). > > From documentation: > * results: > * result-- NUL terminated result which persists until next > * call t

Re: [Spice-devel] [spice-gtk v1 1/3] Improve debug log for preferred compression message

2017-12-20 Thread Christophe Fergeau
On Wed, Dec 20, 2017 at 03:47:25PM +0100, Victor Toso wrote: > To be honest, I wrote this patch 3 times because of this. There > are other places in the source code that we do enum-to-string > conversion and I was thinking in making this better now too. > > But then I thought that I was complicati

Re: [Spice-devel] [nsis v3 2/2] build: Don't add .pdb debug files to the installer

2017-12-20 Thread Christophe Fergeau
On Tue, Dec 19, 2017 at 11:26:00AM -0500, Frediano Ziglio wrote: > > > > The .pdb files contain the debug information for the drivers. They > > increase significantly the size of the installer, so it's better not to > > ship them. > > On the other side bug report will contain much less informatio

Re: [Spice-devel] [PATCH spice-server] main-channel-client: Do not call red_channel_client_begin_send_message if we are not sending a message

2017-12-19 Thread Christophe Fergeau
E_UPCAST(RedRegisteredChannelPipeItem, base)); > +break; > } > -break; > +return; I think this would be easier to read: if (!mcc->priv->initial_channels_list_sent) { return; } main_channel_marshall_registered_channel(rcc, m, SPICE_UPCAST(RedRegist

Re: [Spice-devel] [PATCH spice-server 11/11] inputs-channel: Move spice_server_kbd_leds to InputsChannel

2017-12-19 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Mon, Dec 11, 2017 at 10:28:08AM +, Frediano Ziglio wrote: > This avoids to expose some detail about the channel. > Like other APIs implement it move close to the part that handle > it instead of have everything in reds.c. > > Signed-off-by:

Re: [Spice-devel] [PATCH spice-server 07/11] red-stream: Use mechname for mechname

2017-12-19 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Mon, Dec 11, 2017 at 10:28:04AM +, Frediano Ziglio wrote: > There's no reason to copy mechname into mechlist to use mechlist > instead of mechname. > > Signed-off-by: Frediano Ziglio > --- > server/red-stream.c | 7 ++- > 1

Re: [Spice-devel] [PATCH spice-server 06/11] red-stream: Remove SASL "data" field leak

2017-12-19 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Mon, Dec 11, 2017 at 10:28:03AM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/red-stream.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/server/red-stream.c b/server/red-stream.c > index 45e9

Re: [Spice-devel] [PATCH spice-server 05/11] reds: Remove argument from reds_handle_link

2017-12-19 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Mon, Dec 11, 2017 at 10:28:02AM +, Frediano Ziglio wrote: > RedLinkInfo stores reds in it no need to pass every time. > > Signed-off-by: Frediano Ziglio > --- > server/reds.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 dele

Re: [Spice-devel] [PATCH spice-server 10/11] red-stream: Avoid useless copy of mechlist

2017-12-19 Thread Christophe Fergeau
On Mon, Dec 11, 2017 at 10:28:07AM +, Frediano Ziglio wrote: > The list will persist will the SASL connection is not disposed > (or another sasl_listmech called). Where is this documented? "while the SASL connection.." signature.asc Description: PGP signature ___

Re: [Spice-devel] [PATCH spice-server 08/11] red-stream: Simplify mechname matching

2017-12-19 Thread Christophe Fergeau
On Mon, Dec 11, 2017 at 10:28:05AM +, Frediano Ziglio wrote: > Avoid over complicated matching using quoting and a simple > strstr operation. I would explain the trick with sasl_mechlist prefix/suffix in the commit log. Looks good otherwise > > Signed-off-by: Frediano Ziglio > --- > serve

Re: [Spice-devel] [PATCH spice-server 1/3] dcc: Remove obsolete comment

2017-12-19 Thread Christophe Fergeau
For the series, Acked-by: Christophe Fergeau On Wed, Dec 13, 2017 at 02:01:57PM +, Frediano Ziglio wrote: > RedPipeItem does not contain a link anymore. > > Signed-off-by: Frediano Ziglio > --- > server/dcc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)

Re: [Spice-devel] [PATCH spice-server 01/11] stat-file: Protect flags field with atomic operation

2017-12-19 Thread Christophe Fergeau
On Mon, Dec 11, 2017 at 10:27:58AM +, Frediano Ziglio wrote: > Coverity complaint that this field should be protected by > a mutex as other accesses are with the mutex locked. > Use atomic operation. Not in an hot path in any case. > > Signed-off-by: Frediano Ziglio > --- > server/stat-file.

[Spice-devel] [nsis v3 2/2] build: Don't add .pdb debug files to the installer

2017-12-19 Thread Christophe Fergeau
The .pdb files contain the debug information for the drivers. They increase significantly the size of the installer, so it's better not to ship them. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index df8f7bc..841f01a 100644 --- a/Makefile ++

[Spice-devel] [nsis v3 1/2] Properly quote path to service binaries

2017-12-19 Thread Christophe Fergeau
If these paths are unquoted, and the path contains spaces (C:\Program Files (x86)\...), this could be exploited by putting a binary with a crafted name (C:\Program.exe), leading to privilege escalation as this is a service that is being started. https://www.commonexploits.com/unquoted-service-path

Re: [Spice-devel] [nsis 1/2] Properly quote path to service binaries

2017-12-19 Thread Christophe Fergeau
On Sat, Dec 16, 2017 at 04:14:49AM -0500, Frediano Ziglio wrote: > > > > If these paths are unquoted, and the path contains spaces (C:\Program > > Files (x86)\...), this could be exploited by putting a binary with a > > crafted name (C:\Program.exe), leading to privilege escalation as this > > is

[Spice-devel] [nsis 1/2] Properly quote path to service binaries

2017-12-15 Thread Christophe Fergeau
If these paths are unquoted, and the path contains spaces (C:\Program Files (x86)\...), this could be exploited by putting a binary with a crafted name (C:\Program.exe), leading to privilege escalation as this is a service that is being started. https://www.commonexploits.com/unquoted-service-path

[Spice-devel] [nsis 2/2] build: Don't add .pdb debug files to the installer

2017-12-15 Thread Christophe Fergeau
The .pdb files contain the debug information for the drivers. They increase significantly the size of the installer, so it's better not to ship them. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index df8f7bc..841f01a 100644 --- a/Makefile ++

Re: [Spice-devel] [PATCH spice-gtk v2 0/5] Fix introspection method bindings

2017-12-08 Thread Christophe Fergeau
On Thu, Nov 30, 2017 at 09:33:32AM -0500, Marc-André Lureau wrote: > Hi > > - Original Message - > > On Tue, Sep 19, 2017 at 10:33:56PM +0800, Matthew Francis wrote: > > > Various C functions have not been named correctly to be picked up as bound > > > methods under introspection. This ren

Re: [Spice-devel] [spice-server 1/2] worker: Remove unneeded include

2017-12-08 Thread Christophe Fergeau
On Fri, Dec 08, 2017 at 09:00:34AM -0500, Frediano Ziglio wrote: > > > > Nothing seems to be using openssl in red-worker.c > > > > There are direct call to OpenSSL. Search for "SSL_". > But openssl/ssl.h is included in red-stream.h which is > included everywhere :-( I do not have these here: $

[Spice-devel] [spice-server 1/2] worker: Remove unneeded include

2017-12-08 Thread Christophe Fergeau
Nothing seems to be using openssl in red-worker.c Signed-off-by: Christophe Fergeau --- server/red-worker.c | 1 - 1 file changed, 1 deletion(-) diff --git a/server/red-worker.c b/server/red-worker.c index 08bfdae29..21622b1e3 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -28,7

[Spice-devel] [spice-server 2/2] ssl: Drop support for older OpenSSL versions

2017-12-08 Thread Christophe Fergeau
SSL_OP_NO_COMPRESSION was introduced in OpenSSL_0_9_8k, which is no longer supported. This commit raises the minimum OpenSSL version to 1.0.0, which is also out of support. Signed-off-by: Christophe Fergeau --- configure.ac | 2 +- server/reds.c | 22 +++--- 2 files changed, 4

Re: [Spice-devel] [spice-server] build: Rebuild shared library when symbol file changes

2017-12-07 Thread Christophe Fergeau
nstead of > $(top_srcdir)/server/spice-server.syms ? I copied and pasted what is on the libspice_server_la_LDFLAGS line :) I'll push this as a followup: commit 1ad7f1f4b8498ea0f98bc145576a8ea57d1c74c7 (HEAD -> tls) Author: Christophe Fergeau Date: Thu Dec 7 18:01:58 2017 +0100 build:

[Spice-devel] [spice-server] build: Rebuild shared library when symbol file changes

2017-12-07 Thread Christophe Fergeau
At the moment, changing spice-server.syms to add/remove a new symbol to be exported does not regenerate spice-server.so. This commit added the needed dependency for this to work. Signed-off-by: Christophe Fergeau --- server/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/server

Re: [Spice-devel] [PATCH spice-server] Do not compute stream_id more that necessary

2017-12-07 Thread Christophe Fergeau
On Thu, Dec 07, 2017 at 10:26:57AM -0500, Frediano Ziglio wrote: > > > > Hi, > > > > On Thu, Dec 07, 2017 at 12:26:43PM +, Frediano Ziglio wrote: > > > Reuse already computed value and avoid to compute in every > > > iterations. > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > > se

Re: [Spice-devel] [NSIS] packaging: bump version to 4.3

2017-12-07 Thread Christophe Fergeau
Sure, Acked-by: Christophe Fergeau On Tue, Dec 05, 2017 at 05:31:02PM +0200, Yedidyah Bar David wrote: > Change-Id: I1bfcf05a48d3af8226061f47d76ba64e9a16c61a > Signed-off-by: Yedidyah Bar David > --- > ovirt-guest-tools-iso.spec.in | 5 - > 1 file changed, 4 insertions

Re: [Spice-devel] ps lasso tool is obscure

2017-12-05 Thread Christophe Fergeau
On Fri, Dec 01, 2017 at 05:15:41AM -0500, Frediano Ziglio wrote: > Can we have a bit more details? > Which OS and version is running the VM? Which resolution and color depth? > Which OS and version (Windows x64 SP1 is not a version) is running the > client? Which resolution and color depth? > D

Re: [Spice-devel] [PATCH vdagent v3 1/3] vdagent: add GTK+

2017-12-01 Thread Christophe Fergeau
On Thu, Nov 30, 2017 at 01:06:37PM -0500, Frediano Ziglio wrote: > Yes, the 2 suggestions seem to conflict. I'm not sure about the page > I pointed out. For instance I just checked RHEL 7.4 and there's gtk 3.22 > so is fine, maybe the page is referring at the first release, not sure > if we should

Re: [Spice-devel] [PATCH spice-gtk v2 0/5] Fix introspection method bindings

2017-11-30 Thread Christophe Fergeau
On Tue, Sep 19, 2017 at 10:33:56PM +0800, Matthew Francis wrote: > Various C functions have not been named correctly to be picked up as bound > methods under introspection. This renames them to ensure correct binding. I'm very late to the party, but had you considered using (rename-to NEWNAME) ann

Re: [Spice-devel] Spice-server gobject bindings

2017-11-29 Thread Christophe Fergeau
Hey Matthew, I've finally taken a quick look at these bindings. For some reason I could not build it: ssg-server.c: In function 'ssg_server_class_init': ssg-server.c:630:21: error: 'SSG_TYPE_SPICE_COMPAT_VERSION_T' undeclared (first use in this function); did you mean 'SSG_TYPE_SPICE_PORT_EVENT_

<    6   7   8   9   10   11   12   13   14   15   >