[Spice-devel] [PATCH spice-server 04/10] mjpeg: Reuse realloc instead of implementing it manually

This potentially can also save the copy if there are enough space to resize the buffer in place. Signed-off-by: Frediano Ziglio --- server/mjpeg-encoder.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c

[Spice-devel] [PATCH spice-server 01/10] tests: Check leaks in spice_server_add_ssl_client if invalid connection

Signed-off-by: Frediano Ziglio --- server/tests/Makefile.am | 1 + server/tests/test-leaks.c | 12 2 files changed, 13 insertions(+) diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am index 01e636265..d50c590c3 100644 ---

[Spice-devel] [PATCH spice-server 06/10] red-channel-client: Early check for valid stream

Code tested for the presence of a stream while initializing RedChannelClient. However the check was done too late possibly using the invalid pointer in red_channel_client_config_socket. Signed-off-by: Frediano Ziglio --- Can the stream be NULL? By the way, I think this

[Spice-devel] [PATCH spice-server 00/10] Miscellaneous patches

Not much related one with the other. Exceptions are first 2 (dealing with spice_server_add_ssl_client leak) and red-channel-client ones (dealing with red_channel_client_wait_pipe_item_sent and MarkerPipeItem). Frediano Ziglio (10): tests: Check leaks in spice_server_add_ssl_client if invalid

[Spice-devel] [PATCH spice-server 02/10] reds: Fix leaks if reds_init_client_ssl_connection fails

If a client is able to complete the TLS handshake phase reds_init_client_ssl_connection leaked some memory as the stream is not correctly freed. This also cause the stream to send the SPICE_CHANNEL_EVENT_DISCONNECTED event. Otherwise only SPICE_CHANNEL_EVENT_CONNECTED was sent. Signed-off-by:

Re: [Spice-devel] [PATCH spice-server 16/16] red-worker: Start processing commands as soon as possible

ping > > When the worker is started it could take a while to start > processing commands. > The reason is that the dispatcher handler is called after > the worker so GLib will receive a FALSE answer to both > prepare and check callbacks of the RedWorkerSource causing > GLib to wait till another

[Spice-devel] [PATCH spice-server 03/10] reds-stream: Remove shutdown field

This field was used only by RedChannelClient to mark when the socket was shutdown. This condition can simply be tested by RedChannelClient checking if there's a watch as is the only condition (beside object destroying) where the watch is removed. In any case the shutdown was used to understand if

[Spice-devel] [PATCH spice-server 05/10] syntax-check: Check ENABLE_EXTRA_CHECKS is not used incorrectly

Usually configuration macros are defined to 0 or undefined. For this reason these macros are sometimes checked using #if and sometimes with #ifndef/#ifdef. As this macro is always defined with 0 or 1 it makes no sense to check if defined or not so check the code to avoid this mistake.

[Spice-devel] [PATCH spice-server 09/10] red-channel-client: Simplify red_channel_client_wait_pipe_item_sent

Instead of inserting the marker after the item (the tail of the queue is the first item to send) and then have to wait again if for the specific item place the marker before the item so waiting for the marker to be sent assure that we sent also the item. This avoids having to call

[Spice-devel] [PATCH spice-server 10/10] tests: Avoid to disable all deprecation warnings just for expect functions

In case GLib don't provide these functions we use replacements so there's no need to have a warning if these functions are called. This potentially capture other compatibility issues in the tests that would be ignored having all deprecation warnings disabled. Tested with GLib 2.28 and 2.52.

[Spice-devel] [PATCH spice-server 08/10] red-channel-client: Simplify red_channel_client_wait_pipe_item_sent loop

Avoid repeating the same code twice. red_channel_client_receive and red_channel_client_send already check if client is blocked, no need to check manually. Put the check for the loop inside it to avoid this duplication. Signed-off-by: Frediano Ziglio ---

Re: [Spice-devel] [PATCH spice-protocol v3 0/5] Protocol extension to support sending display data as video streams

ping > > This protocol is intended for guest machines with assigned hardware > devices to send encoded video and other informations that will be > handled as a normal display by Spice. > > New GPU devices are capable to capture and encode video in an > efficient way. As the GPU in this case is

[Spice-devel] [PATCH spice-server 0/3] Remove some leaks in test-gst test

This helps detecting possible leaks using leak detection tools like Valgrind or Address Sanitizer. Frediano Ziglio (3): test-gst: Remove useless check test-gst: Remove options parsing leaks test-gst: Free pipelines server/tests/test-gst.c | 38 +++--- 1

[Spice-devel] [PATCH spice-server 2/3] test-gst: Remove options parsing leaks

These leaks are detected for instance by address sanitizer. Signed-off-by: Frediano Ziglio --- server/tests/test-gst.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/server/tests/test-gst.c b/server/tests/test-gst.c index

[Spice-devel] [PATCH spice-server 3/3] test-gst: Free pipelines

Remove some leaks. Signed-off-by: Frediano Ziglio --- server/tests/test-gst.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/server/tests/test-gst.c b/server/tests/test-gst.c index 224a891b1..ca187d527 100644 --- a/server/tests/test-gst.c +++

[Spice-devel] [PATCH spice-server 1/3] test-gst: Remove useless check

encoder_name is never NULL as already initialized with "mjpeg" value. Signed-off-by: Frediano Ziglio --- server/tests/test-gst.c | 5 - 1 file changed, 5 deletions(-) diff --git a/server/tests/test-gst.c b/server/tests/test-gst.c index d4ebaacf6..40f738d78 100644 ---

[Spice-devel] [PATCH spice-common] codegen: Make the compiler work about better way to write unaligned memory

Instead of assuming that the system can safely do unaligned access to memory use packed structures to allow the compiler generate best code possible. For instance ARM7 can use unaligned access but not for 64 bit numbers (currently these accesses are emulated by Linux kernel with obvious

[Spice-devel] [PATCH spice-server 2/2] red-qxl: Reuse red_qxl_async_complete

Now that the function is a wrapper reuse it. Signed-off-by: Frediano Ziglio --- server/red-qxl.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/red-qxl.c b/server/red-qxl.c index 5815e4406..ef1f662df 100644 --- a/server/red-qxl.c +++

[Spice-devel] [PATCH spice-server 1/2] red-qxl: Remove AsyncCommand

This structure was used to store the cookie for the async reply and the message for the generic async callback. Most async messages do not require extra action beside sending back the cookie for the reply so instead of using a switch inline the replies and remove store the cookie directory instead

[Spice-devel] [PATCH spice-server 2/2] red-channel-client: Prevent too tight loop waiting for ACKs

RedChannelClient has a "handle-acks" feature. If this feature is enabled after the configured number of messages it waits for an ACK. If is waiting for an ACK it stops sending messages. However the write notification was not disabled causing the loop event to always trigger as the socket in this

[Spice-devel] [PATCH spice-server 1/2] red-channel-client: Introduce some helpers to update watch event mask

Signed-off-by: Frediano Ziglio --- server/red-channel-client.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/server/red-channel-client.c b/server/red-channel-client.c index d9333ba6f..eab1d593d 100644 ---

Re: [Spice-devel] [PATCH spice-server v2] docs: Add some documentation on spice threading model

ping > > Signed-off-by: Frediano Ziglio > --- > .gitignore | 1 + > docs/Makefile.am | 6 ++-- > docs/spice_threading_model.txt | 74 > ++ > 3 files changed, 79 insertions(+), 2 deletions(-) >

Re: [Spice-devel] [PATCH spice-server 01/10] tests: Check leaks in spice_server_add_ssl_client if invalid connection

Since you needed a comment in the code, I guess the commit log could be a bit more detailed as well... Which would probably allow to have a shorter short log. On Mon, Sep 11, 2017 at 11:15:38AM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- >

Re: [Spice-devel] [PATCH spice-server 02/10] reds: Fix leaks if reds_init_client_ssl_connection fails

On Mon, Sep 11, 2017 at 11:15:39AM +0100, Frediano Ziglio wrote: > If a client is able to complete the TLS handshake phase > reds_init_client_ssl_connection leaked some memory > as the stream is not correctly freed. > This also cause the stream to send the SPICE_CHANNEL_EVENT_DISCONNECTED

[Spice-devel] [PATCH spice-server 10/11] Use GLib memory functions for RedsMigSpice::cert_subject

Signed-off-by: Frediano Ziglio --- server/main-channel.c | 4 ++-- server/reds.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/main-channel.c b/server/main-channel.c index d1fe8bd7d..eca857f6b 100644 --- a/server/main-channel.c +++

Re: [Spice-devel] [PATCH spice-server 1/2] red-channel-client: Introduce some helpers to update watch event mask

> > Why ? > > Christophe for 2/2. Maybe I should add "These helpers will be reused by following patch." ? Frediano > > On Mon, Sep 11, 2017 at 01:29:05PM +0100, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > server/red-channel-client.c | 27

[Spice-devel] [PATCH spice-server 06/11] reds: Use GLib memory functions for migration data

Signed-off-by: Frediano Ziglio --- server/reds.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/reds.c b/server/reds.c index 5d7f39621..0b9ffa8da 100644 --- a/server/reds.c +++ b/server/reds.c @@ -590,7 +590,7 @@ void

[Spice-devel] [PATCH spice-server 04/11] reds: Start using GLib memory functions

Signed-off-by: Frediano Ziglio --- server/reds.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/server/reds.c b/server/reds.c index 01e8c5499..3782e6404 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2051,7 +2051,7 @@ static

[Spice-devel] [PATCH spice-server 07/11] Use GLib memory functions for RedsMigSpice::host

Signed-off-by: Frediano Ziglio --- server/main-channel.c | 4 ++-- server/reds.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/main-channel.c b/server/main-channel.c index 9d2590aa8..d1fe8bd7d 100644 --- a/server/main-channel.c +++

[Spice-devel] [PATCH spice-server 05/11] reds: Use GLib memory functions for link

Signed-off-by: Frediano Ziglio --- server/reds.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/reds.c b/server/reds.c index 3782e6404..5d7f39621 100644 --- a/server/reds.c +++ b/server/reds.c @@ -345,7 +345,7 @@ static void

[Spice-devel] [PATCH spice-server 01/11] Start using GLib memory allocation

Start reducing the usage of spice_new*/spice_malloc allocations. They were designed in a similar way to GLib ones. Now that we use GLib make sense to remove them. However the versions we support for GLib can use different memory allocators so we have to match g_free with GLib allocations and

[Spice-devel] [PATCH spice-server 02/11] image-encoders: Use GLib memory functions

Signed-off-by: Frediano Ziglio --- server/image-encoders.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/image-encoders.c b/server/image-encoders.c index cf6689683..880972e70 100644 --- a/server/image-encoders.c +++

[Spice-devel] [PATCH spice-server 03/11] sound: Use GLib memory functions

Signed-off-by: Frediano Ziglio --- server/sound.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/sound.c b/server/sound.c index de51a4670..9073626cd 100644 --- a/server/sound.c +++ b/server/sound.c @@ -795,7 +795,7 @@

[Spice-devel] [PATCH spice-server 08/11] reds: Use GLib memory functions for RedServerConfig::mig_spice

Signed-off-by: Frediano Ziglio --- server/reds.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/reds.c b/server/reds.c index acb25dd9f..9b573987a 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2964,7 +2964,7 @@ static void

[Spice-devel] [PATCH spice-server 09/11] reds: Use GLib memory functions for RedServerConfig

Signed-off-by: Frediano Ziglio --- server/reds.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/reds.c b/server/reds.c index 9b573987a..8906708d1 100644 --- a/server/reds.c +++ b/server/reds.c @@ -3501,7 +3501,7 @@ SPICE_GNUC_VISIBLE

[Spice-devel] [PATCH spice-server 11/11] lz4-encoder: Use GLib memory functions

Signed-off-by: Frediano Ziglio --- server/lz4-encoder.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c index fe736d272..ca9b1b825 100644 --- a/server/lz4-encoder.c +++

[Spice-devel] [PATCH spice-server 00/11] Start using GLib memory allocation

Start reducing the usage of spice_new*/spice_malloc* allocations. They were designed in a similar way to GLib ones. Now that we use GLib make sense to remove them. However the versions we support for GLib can use different memory allocators so we have to match g_free with GLib allocations and

Re: [Spice-devel] [PATCH spice-server 1/2] red-channel-client: Introduce some helpers to update watch event mask

Why ? Christophe On Mon, Sep 11, 2017 at 01:29:05PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/red-channel-client.c | 27 --- > 1 file changed, 16 insertions(+), 11 deletions(-) > > diff --git

Re: [Spice-devel] [PATCH spice-server 02/10] reds: Fix leaks if reds_init_client_ssl_connection fails

> > On Mon, Sep 11, 2017 at 11:15:39AM +0100, Frediano Ziglio wrote: > > If a client is able to complete the TLS handshake phase > > reds_init_client_ssl_connection leaked some memory > > as the stream is not correctly freed. > > This also cause the stream to send the

Re: [Spice-devel] [PATCH spice-server 05/10] syntax-check: Check ENABLE_EXTRA_CHECKS is not used incorrectly

On Mon, Sep 11, 2017 at 11:15:42AM +0100, Frediano Ziglio wrote: > Usually configuration macros are defined to 0 or undefined. > For this reason these macros are sometimes checked using #if > and sometimes with #ifndef/#ifdef. > As this macro is always defined with 0 or 1 it makes no sense > to

Re: [Spice-devel] [PATCH spice-server 2/3] test-gst: Remove options parsing leaks

On Mon, Sep 11, 2017 at 09:12:17AM +0100, Frediano Ziglio wrote: > These leaks are detected for instance by address sanitizer. More details about what these leaks are would be welcome... I can guess what these are, but the commit log is really where this belongs. > > Signed-off-by: Frediano

Re: [Spice-devel] [PATCH spice-server 03/10] reds-stream: Remove shutdown field

Acked-by: Christophe Fergeau On Mon, Sep 11, 2017 at 11:15:40AM +0100, Frediano Ziglio wrote: > This field was used only by RedChannelClient to mark when the > socket was shutdown. This condition can simply be tested by > RedChannelClient checking if there's a watch as is

Re: [Spice-devel] [PATCH spice-server 03/10] reds-stream: Remove shutdown field

> > > Acked-by: Christophe Fergeau > > On Mon, Sep 11, 2017 at 11:15:40AM +0100, Frediano Ziglio wrote: > > This field was used only by RedChannelClient to mark when the > > socket was shutdown. This condition can simply be tested by > > RedChannelClient checking if

Re: [Spice-devel] [PATCH spice-server 02/10] reds: Fix leaks if reds_init_client_ssl_connection fails

On Mon, Sep 11, 2017 at 11:47:10AM -0400, Frediano Ziglio wrote: > > > > On Mon, Sep 11, 2017 at 11:15:39AM +0100, Frediano Ziglio wrote: > > > If a client is able to complete the TLS handshake phase > > > reds_init_client_ssl_connection leaked some memory > > > as the stream is not correctly

Re: [Spice-devel] [PATCH spice-server 05/10] syntax-check: Check ENABLE_EXTRA_CHECKS is not used incorrectly

On Mon, Sep 11, 2017 at 11:48:47AM -0400, Frediano Ziglio wrote: > > > > On Mon, Sep 11, 2017 at 11:15:42AM +0100, Frediano Ziglio wrote: > > > Usually configuration macros are defined to 0 or undefined. > > > For this reason these macros are sometimes checked using #if > > > and sometimes with

Re: [Spice-devel] [PATCH spice-server 1/3] test-gst: Remove useless check

On Mon, Sep 11, 2017 at 09:12:16AM +0100, Frediano Ziglio wrote: > encoder_name is never NULL as already initialized with "mjpeg" value. Note that this is going to be changed in patch 3/3 Christophe > > Signed-off-by: Frediano Ziglio > --- > server/tests/test-gst.c | 5

Re: [Spice-devel] [PATCH spice-server v2] docs: Add some documentation on spice threading model

On Mon, Sep 11, 2017 at 03:34:28AM -0400, Frediano Ziglio wrote: > ping I believe I was fine with the v1 except for the small comments I made on it, still looks good after a quick glance. Acked-by: Christophe Fergeau ___

Re: [Spice-devel] [PATCH spice-server 05/10] syntax-check: Check ENABLE_EXTRA_CHECKS is not used incorrectly

> > On Mon, Sep 11, 2017 at 11:15:42AM +0100, Frediano Ziglio wrote: > > Usually configuration macros are defined to 0 or undefined. > > For this reason these macros are sometimes checked using #if > > and sometimes with #ifndef/#ifdef. > > As this macro is always defined with 0 or 1 it makes no

Re: [Spice-devel] [PATCH spice-server 10/10] tests: Avoid to disable all deprecation warnings just for expect functions

On Mon, Sep 11, 2017 at 11:15:47AM +0100, Frediano Ziglio wrote: > In case GLib don't provide these functions we use replacements so > there's no need to have a warning if these functions are called. > This potentially capture other compatibility issues in the tests > that would be ignored having

Re: [Spice-devel] [PATCH spice-server 1/2] red-channel-client: Introduce some helpers to update watch event mask

On Mon, Sep 11, 2017 at 11:41:17AM -0400, Frediano Ziglio wrote: > > > > > Why ? > > > > Christophe > > for 2/2. > > Maybe I should add > > "These helpers will be reused by following patch." Yup, something like that would be useful, thanks. > > > > On Mon, Sep 11, 2017 at 01:29:05PM

Re: [Spice-devel] [PATCH spice-server 16/16] red-worker: Start processing commands as soon as possible

On Mon, Sep 11, 2017 at 04:03:18AM -0400, Frediano Ziglio wrote: > ping > > > > > When the worker is started it could take a while to start > > processing commands. > > The reason is that the dispatcher handler is called after > > the worker so GLib will receive a FALSE answer to both > >

Re: [Spice-devel] [PATCH spice-html5] cursor: Add support for mono cursor type

Hi Tomáš, I am concerned about the source and license for the new code. On 09/06/2017 03:50 AM, Tomáš Bohdálek wrote: [snip] > diff --git a/thirdparty/monocursor.js b/thirdparty/monocursor.js > new file mode 100644 > index 000..5df6da9 > --- /dev/null > +++ b/thirdparty/monocursor.js > @@

Re: [Spice-devel] [PATCH spice-server 04/10] mjpeg: Reuse realloc instead of implementing it manually

On Mon, Sep 11, 2017 at 11:15:41AM +0100, Frediano Ziglio wrote: > This potentially can also save the copy if there are enough > space to resize the buffer in place. "if there is" Acked-by: Christophe Fergeau > > Signed-off-by: Frediano Ziglio > --- >

Re: [Spice-devel] [PATCH spice-server 08/10] red-channel-client: Simplify red_channel_client_wait_pipe_item_sent loop

On Mon, Sep 11, 2017 at 11:15:45AM +0100, Frediano Ziglio wrote: > Avoid repeating the same code twice. > red_channel_client_receive and red_channel_client_send already > check if client is blocked, no need to check manually. > Put the check for the loop inside it to avoid this duplication. Not

Re: [Spice-devel] [PATCH spice-server 03/10] reds-stream: Remove shutdown field

On Mon, Sep 11, 2017 at 11:56:45AM -0400, Frediano Ziglio wrote: > > > > > > Acked-by: Christophe Fergeau > > > > On Mon, Sep 11, 2017 at 11:15:40AM +0100, Frediano Ziglio wrote: > > > This field was used only by RedChannelClient to mark when the > > > socket was shutdown.

Re: [Spice-devel] [PATCH spice-server 3/3] test-gst: Free pipelines

On Mon, Sep 11, 2017 at 09:12:18AM +0100, Frediano Ziglio wrote: > Remove some leaks. I would not really call these a leak, it's just memory which is still reachable when the program exits, but not explicitly freed. Acked-by: Christophe Fergeau > > Signed-off-by: