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

On Thu, Feb 15, 2018 at 05:06:19PM +0100, Christophe de Dinechin 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

Re: [Spice-devel] [PATCH v4 08/12] Add guidelines about warnings

On Thu, Feb 15, 2018 at 05:06:21PM +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 > > Changes since v3: > - Put 'controversial'

Re: [Spice-devel] [PATCH spice-streaming-agent v2 4/4] log_binary is really a boolean

> > On 15 Feb 2018, at 11:00, Frediano Ziglio wrote: > > > > From: Christophe de Dinechin > > > > Signed-off-by: Christophe de Dinechin > > --- > > Change since v1: > > - do not clash with possible short 'b' option. > > --- > >

Re: [Spice-devel] [PATCH v4 10/12] Integrate suggestions from Christophe Fergeau

The shortlog is not really good.. On Thu, Feb 15, 2018 at 05:06:23PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > This attempts to find a wording that takes into account the existing > practice, notably in the server. > > Signed-off-by:

Re: [Spice-devel] [PATCH spice-server v2 1/2] stream-device: handle cursor from device

> > > On 9 Feb 2018, at 17:59, Frediano Ziglio wrote: > > > > > > + > > +// read part of the message till we get all > > +if (dev->msg_len < dev->hdr.size) { > > +dev->msg = g_realloc(dev->msg, dev->hdr.size); > > +

Re: [Spice-devel] [PATCH v4 04/12] Rephrase assertion section

On Thu, Feb 15, 2018 at 05:06:17PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Changes since v3: > - Integrate comments about performance impact and solution > - Integrate comments about impact of a failed assertion > - Add prohibition against

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

On Thu, Feb 15, 2018 at 05:06:14PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > There are two use cases for this .clang-format: > > 1. Local reformatting of source code, e.g. using Emacs clang-format.el > package. >This is basically a wawy to

Re: [Spice-devel] [PATCH v4 12/12] Whitespace adjustment guideline

On Thu, Feb 15, 2018 at 05:06:25PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Change since v3: > - This particular aspect is controversial, so it was put in a separate patch. This has been nack'ed by multiple people already, please don't resend

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

> On 16 Feb 2018, at 10:07, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 05:06:14PM +0100, Christophe de Dinechin wrote: >> From: Christophe de Dinechin >> >> There are two use cases for this .clang-format: >> >> 1. Local reformatting of

[Spice-devel] [PATCH spice-gtk v2 0/4] Add spice+tls:// uri form

From: Marc-André Lureau Hi, Here are a few patches related to spice schemes URI handling. They introduce a spice+tls:// form (see related man page update and patch for details). cheers v2: - add a patch to report warning with original uri for clarity - accept

[Spice-devel] [PATCH spice-gtk v2 2/4] uri: learn to parse spice+tls:// form

From: Marc-André Lureau spice:// has a weird scheme encoding, where it can accept both plain and tls ports with URI query parameters. However, it's not very convenient nor very common to use (who really want to mix plain & tls channels?). Instead, let's introduce

Re: [Spice-devel] [PATCH v4 04/12] Rephrase assertion section

On Thu, Feb 15, 2018 at 05:06:17PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Changes since v3: > - Integrate comments about performance impact and solution > - Integrate comments about impact of a failed assertion > - Add prohibition against

Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins

> > > On 6 Feb 2018, at 10:51, Frediano Ziglio wrote: > > > >>> > >>> On 5 Feb 2018, at 15:29, Lukáš Hrázký wrote: > >>> > >>> The static registration (that is, having a static list of pointers to > >>> static plugin variables and calling a generic

[Spice-devel] [PATCH spice-gtk v2 3/4] uri: generate spice://host:port or spice+tls://host:port

From: Marc-André Lureau Signed-off-by: Marc-André Lureau --- src/spice-session.c | 31 +++ tests/session.c | 20 ++-- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git

Re: [Spice-devel] [PATCH v4 04/12] Rephrase assertion section

> On 16 Feb 2018, at 10:12, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 05:06:17PM +0100, Christophe de Dinechin wrote: >> From: Christophe de Dinechin >> >> Changes since v3: >> - Integrate comments about performance impact and solution >>

Re: [Spice-devel] [PATCH spice-gtk v2 2/4] uri: learn to parse spice+tls:// form

On Fri, Feb 16, 2018 at 11:13:06AM +0100, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > spice:// has a weird scheme encoding, where it can accept both plain > and tls ports with URI query parameters. However, it's not very > convenient nor very

[Spice-devel] [PATCH spice-gtk v2 1/4] session: warn with the original URI

From: Marc-André Lureau Before: (lt-spicy:18372): GSpice-WARNING **: 10:57:21.246: Double set of 'port' in URI 'spice://foo' After: (lt-spicy:18654): GSpice-WARNING **: 10:57:21.246: Double set of 'port' in URI 'spice://foo:23?port=24' Signed-off-by: Marc-André

Re: [Spice-devel] [PATCH v4 04/12] Rephrase assertion section

On Fri, Feb 16, 2018 at 10:40:58AM +0100, Christophe de Dinechin wrote: > > > > On 16 Feb 2018, at 10:12, Christophe Fergeau wrote: > > > > On Thu, Feb 15, 2018 at 05:06:17PM +0100, Christophe de Dinechin wrote: > >> From: Christophe de Dinechin > >>

Re: [Spice-devel] [PATCH spice-gtk v2 1/4] session: warn with the original URI

> > From: Marc-André Lureau > > Before: > (lt-spicy:18372): GSpice-WARNING **: 10:57:21.246: Double set of 'port' in > URI 'spice://foo' > > After: > (lt-spicy:18654): GSpice-WARNING **: 10:57:21.246: Double set of 'port' in > URI 'spice://foo:23?port=24' > >

[Spice-devel] [PATCH spice-streaming-agent] build: Execute tests while packaging RPM file

Signed-off-by: Frediano Ziglio --- spice-streaming-agent.spec.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in index 99b1f52..d137840 100644 --- a/spice-streaming-agent.spec.in +++

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

> On 16 Feb 2018, at 10:05, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 05:06:19PM +0100, Christophe de Dinechin wrote: >> From: Christophe de Dinechin >> >> Signed-off-by: Christophe de Dinechin >> --- >>

[Spice-devel] [PATCH spice-gtk v2 4/4] tests: add spice+tls:// tests

From: Marc-André Lureau They couldn't not be introduced before, because the test needs both parsing and generation. Signed-off-by: Marc-André Lureau --- tests/session.c | 24 +++- 1 file changed, 23 insertions(+), 1

Re: [Spice-devel] [PATCH 2/2] Eliminate signed/unsigned warning

> > From: Christophe de Dinechin > > Without this, GCC complains about signed / unsigned comparisons: > > mjpeg-fallback.cpp:121:24: warning: comparison between signed and unsigned > integer expressions [-Wsign-compare] > if (win_info.width != last_width ||

Re: [Spice-devel] [PATCH 04/17] Do not create std::string for constants

> > From: Christophe de Dinechin > > As per e-mail discussions (https://patchwork.freedesktop.org/patch/200629/), > using std::string for C-style string constants is against C++ good practices, > see >

Re: [Spice-devel] [PATCH 1/2] Add missing header

> > From: Christophe de Dinechin > > Without this header, clang complains with messages such as: > > concrete-agent.cpp:62:37: error: invalid operands to binary expression > ('const std::string' (aka 'const basic_string allocator >') and 'const char

Re: [Spice-devel] [PATCH 05/17] Use RAII to cleanup stream in case of exception or return

> > From: Christophe de Dinechin > > Get rid of C-style 'goto done' in do_capture. > Get rid of global streamfd, pass it around (cleaned up in later patch) > Fixes a race condition, make sure we only use stream after opening > > Signed-off-by: Christophe de Dinechin

Re: [Spice-devel] [PATCH 02/17] log_binary is really a boolean

> > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > src/spice-streaming-agent.cpp | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/spice-streaming-agent.cpp

Re: [Spice-devel] [PATCH 05/17] Use RAII to cleanup stream in case of exception or return

> On 16 Feb 2018, at 17:40, Frediano Ziglio wrote: > >> >> From: Christophe de Dinechin >> >> Get rid of C-style 'goto done' in do_capture. >> Get rid of global streamfd, pass it around (cleaned up in later patch) >> Fixes a race condition, make sure

[Spice-devel] [PATCH 00/17] WIP: Refactor the streaming agent towards a more standard C++ style

From: Christophe de Dinechin The streaming agent started as C code. This series converts the style to something that is more usual for C++, notably: - Adds encapsulation and RAII for resources such as the stream - Splits functionality into several classes with clear roles -

[Spice-devel] [PATCH 02/17] log_binary is really a boolean

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp index

[Spice-devel] [PATCH 09/17] Reorder headers according to style guide

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp index

[Spice-devel] [PATCH 12/17] Convert message writing from C style to C++ style

From: Christophe de Dinechin - The Stream class now deals with locking and sending messages - The Message<> template class deals with the general writing mechanisms - Three classes, FormatMessage, FrameMessage and X11CursorMessage represent individual messages The various

[Spice-devel] [PATCH 04/17] Do not create std::string for constants

From: Christophe de Dinechin As per e-mail discussions (https://patchwork.freedesktop.org/patch/200629/), using std::string for C-style string constants is against C++ good practices, see

[Spice-devel] [PATCH 08/17] Use C++ style for cursor message initialization instead of C style

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 47 +-- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 16/17] Remove client_codecs global variable, moved inside the 'Stream' class

From: Christophe de Dinechin This is not a really nice abstraction at this point, but still a step in the right way Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 14 +- 1 file changed, 9 insertions(+), 5

Re: [Spice-devel] [PATCH 06/17] Replace inefficient C-style initialization with C++-style

> > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > src/spice-streaming-agent.cpp | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 10/17] Since we use a namespace, simplify name of local classes

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 05/17] Use RAII to cleanup stream in case of exception or return

From: Christophe de Dinechin Get rid of C-style 'goto done' in do_capture. Get rid of global streamfd, pass it around (cleaned up in later patch) Fixes a race condition, make sure we only use stream after opening Signed-off-by: Christophe de Dinechin

[Spice-devel] [PATCH 07/17] Get rid of C-style memset initializations, use C++ style aggregates

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 47 ++- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 11/17] Move read, write and locking into the 'Stream' class

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 86 +++ 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 06/17] Replace inefficient C-style initialization with C++-style

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp index

[Spice-devel] [PATCH 01/17] Add missing header

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/concrete-agent.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp index 891c09b..ac37788 100644 --- a/src/concrete-agent.cpp

[Spice-devel] [PATCH 14/17] Create a class encapsulating the X11 display cursor capture

From: Christophe de Dinechin This class will need to be moved to a separate file in a later patch. This is done in two steps to make the evolution of the code easier to track, as well as to facilitate later 'git bisect' Signed-off-by: Christophe de Dinechin

[Spice-devel] [PATCH 15/17] Create FrameLog class to abstract logging of frames

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 99 ++- 1 file changed, 60 insertions(+), 39 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 13/17] Add more meaningful syslog reporting

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/spice-streaming-agent.cpp | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/spice-streaming-agent.cpp

[Spice-devel] [PATCH 17/17] Move the capture loop in the ConcreteAgent, get rid of global agent variable

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/concrete-agent.hpp| 4 src/spice-streaming-agent.cpp | 14 ++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/concrete-agent.hpp

[Spice-devel] [PATCH 03/17] Eliminate signed/unsigned warning

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/mjpeg-fallback.cpp| 2 +- src/spice-streaming-agent.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mjpeg-fallback.cpp

Re: [Spice-devel] [PATCH 06/17] Replace inefficient C-style initialization with C++-style

> On 16 Feb 2018, at 17:43, Frediano Ziglio wrote: > >> >> From: Christophe de Dinechin >> >> Signed-off-by: Christophe de Dinechin >> --- >> src/spice-streaming-agent.cpp | 4 +--- >> 1 file changed, 1 insertion(+), 3

[Spice-devel] [PATCH] build: Add autogen.sh convenience script

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- autogen.sh | 9 + 1 file changed, 9 insertions(+) create mode 100755 autogen.sh diff --git a/autogen.sh b/autogen.sh new file mode 100755 index 000..afa3e39 ---

[Spice-devel] [PATCH 0/2] Warning and build error fixes

From: Christophe de Dinechin This series eliminates warnings reported by GCC and instantiation failures reported by clang due to missing header. Christophe de Dinechin (2): Add missing header Eliminate signed/unsigned warning src/concrete-agent.cpp| 1 +

[Spice-devel] [PATCH 2/2] Eliminate signed/unsigned warning

From: Christophe de Dinechin Without this, GCC complains about signed / unsigned comparisons: mjpeg-fallback.cpp:121:24: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (win_info.width != last_width || win_info.height !=

[Spice-devel] [PATCH 1/2] Add missing header

From: Christophe de Dinechin Without this header, clang complains with messages such as: concrete-agent.cpp:62:37: error: invalid operands to binary expression ('const std::string' (aka 'const basic_string') and 'const char *')

Re: [Spice-devel] [PATCH] build: Add autogen.sh convenience script

On Fri, Feb 16, 2018 at 04:18:19PM +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > autogen.sh | 9 + > 1 file changed, 9 insertions(+) > create mode 100755 autogen.sh > >

Re: [Spice-devel] [PATCH spice-streaming-agent] build: Execute tests while packaging RPM file

> On 16 Feb 2018, at 12:51, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > --- > spice-streaming-agent.spec.in | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in > index

Re: [Spice-devel] [PATCH spice-server v3 2/2] stream-device: Implement mouse movement

On Tue, 2018-02-13 at 13:58 +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/stream-device.c | 38 +- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/server/stream-device.c

Re: [Spice-devel] [PATCH spice-server v3 1/2] stream-device: handle cursor from device

On Tue, 2018-02-13 at 13:58 +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/stream-device.c | 165 > ++--- > 1 file changed, 158 insertions(+), 7 deletions(-) > > diff --git a/server/stream-device.c

Re: [Spice-devel] [PATCH spice-server v5 1/8] stream-device: Avoid device to get stuck if multiple messages are batched

On Tue, 2018-02-13 at 15:54 +, Frediano Ziglio wrote: > If messages are sent together by the agent the device is reading > only part of the data. This cause Qemu to not poll for new data and > stream_device_read_msg_from_dev won't be called again. > This can cause a stall. To avoid this

Re: [Spice-devel] [PATCH spice-server v3 1/2] stream-device: handle cursor from device

> On Tue, 2018-02-13 at 13:58 +, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > server/stream-device.c | 165 > > ++--- > > 1 file changed, 158 insertions(+), 7 deletions(-) > > > > diff --git

Re: [Spice-devel] [PATCH spice-server v3 1/2] stream-device: handle cursor from device

On Fri, 2018-02-16 at 17:17 -0500, Frediano Ziglio wrote: > > On Tue, 2018-02-13 at 13:58 +, Frediano Ziglio wrote: > > > Signed-off-by: Frediano Ziglio > > > --- > > > server/stream-device.c | 165 > > > ++--- > > > 1 file