>
> red_channel_disconnect_if_pending_send() and red_channel_wait_all_sent() are
> always called together, we can remove one of the 2 methods.
>
> Signed-off-by: Christophe Fergeau
> ---
> Only change since v1 is an added comment to red_channel_wait_all_sent()
> to explicitly indicate it's going
Signed-off-by: Christophe Fergeau
---
No changes since v1
server/red-channel.c | 10 --
server/red-channel.h | 6 --
2 files changed, 16 deletions(-)
diff --git a/server/red-channel.c b/server/red-channel.c
index b8f4f54e8..b4a92340f 100644
--- a/server/red-channel.c
+++ b/server/r
There is exactly one user in RedChannel, and this can be reimplemented
using already public RedChannelClient API. No need for an extra
function very specialized function with a not great name.
This commit thus removes one method from RedChannelClient public API,
and replaces it with an equivalent
red_channel_disconnect_if_pending_send() and red_channel_wait_all_sent() are
always called together, we can remove one of the 2 methods.
Signed-off-by: Christophe Fergeau
---
Only change since v1 is an added comment to red_channel_wait_all_sent()
to explicitly indicate it's going to disconnect sl
Acked-by: Christophe Fergeau
On Tue, Sep 12, 2017 at 12:52:37PM +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
Instead of having MarkerPipeItem pointing to an external variable with
the possibility to forget to reset it and have a dangling pointer use
reference counting to keep the item and mark the item when sent.
This item is placed into the queue to understand when it was sent. The
current implementation
On Tue, Sep 12, 2017 at 08:30:40AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 11, 2017 at 11:15:46AM +0100, Frediano Ziglio wrote:
> > > 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 specif
On Tue, Sep 12, 2017 at 03:37:43AM -0400, Frediano Ziglio wrote:
> >
> > 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 dispatc
Avoid repeating the same code twice.
red_channel_client_send send the item pending (or the a part of it). If
there are no item pending the function does nothing (so checking for
blocked channel is useless). Also red_channel_client_send is already
called from red_channel_client_push checking for cha
>
> On Mon, Sep 11, 2017 at 11:15:46AM +0100, Frediano Ziglio wrote:
> > 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 b
On Mon, Sep 11, 2017 at 06:28:40PM +0200, Christophe Fergeau wrote:
> 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
I guess no answer means this is i
On Mon, Sep 11, 2017 at 11:15:46AM +0100, Frediano Ziglio wrote:
> 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
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.
Signed
>
> On Tue, Sep 12, 2017 at 07:19:32AM -0400, Frediano Ziglio wrote:
> > About checking for pointer assuming you have to check for NULL
> > would mean the NULL is not 0 which would mean that
> > memset(structure_ptr, 0, sizeof(structure)) cannot set pointer
> > to NULL which would mean that assumi
On Tue, Sep 12, 2017 at 07:28:59AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 06:55:41AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > > > > In this case you are introducing a regression, current code is
> > >
>
> On Tue, Sep 12, 2017 at 06:55:41AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > > > In this case you are introducing a regression, current code is
> > > > designed to support this already. I cannot surely say that this
> > > >
On Tue, Sep 12, 2017 at 07:19:32AM -0400, Frediano Ziglio wrote:
> About checking for pointer assuming you have to check for NULL
> would mean the NULL is not 0 which would mean that
> memset(structure_ptr, 0, sizeof(structure)) cannot set pointer
> to NULL which would mean that assuming pointer in
>
> On Tue, Sep 12, 2017 at 05:44:23AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Sep 12, 2017 at 04:11:57AM -0400, Frediano Ziglio wrote:
> > > > >
> > > > > On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> > > > > > >
> > > > > > > On Tue, Sep 12, 2017 at 03:19:12AM -
Acked-by: Christophe Fergeau
On Tue, Sep 12, 2017 at 10:45:08AM +0100, Frediano Ziglio wrote:
> Command line options are not freed at the end of the program.
> These are detected as leaks by leak detector tools like address sanitizer.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/te
On Tue, Sep 12, 2017 at 06:56:34AM -0400, Frediano Ziglio wrote:
> >
> > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > defined this way, otherwise this will break display-channel.c
> > compilation.
> > This is different from most AC_DEFINE() preprocessor constants which
On Tue, Sep 12, 2017 at 06:55:41AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > > In this case you are introducing a regression, current code is
> > > designed to support this already. I cannot surely say that this
> > > patch is an imp
>
> >
> > On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > > In this case you are introducing a regression, current code is
> > > designed to support this already. I cannot surely say that this
> > > patch is an improvement.
> >
> > By that reasoning, any patch which makes so
>
> Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> defined this way, otherwise this will break display-channel.c
> compilation.
> This is different from most AC_DEFINE() preprocessor constants which are
> usually either set to 1, or unset.
>
> This commit switches ENABLE_E
>
> On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> > In this case you are introducing a regression, current code is
> > designed to support this already. I cannot surely say that this
> > patch is an improvement.
>
> By that reasoning, any patch which makes some functions stat
Acked-by: Christophe Fergeau
On Tue, Sep 12, 2017 at 09:03:16AM +0100, Frediano Ziglio wrote:
> If a client is unable to complete the TLS handshake phase
> reds_init_client_ssl_connection leaked some memory as the stream is not
> correctly freed.
> This also causes the stream to send the SPICE_C
On Tue, Sep 12, 2017 at 09:03:15AM +0100, Frediano Ziglio wrote:
> Currently is possible to trigger a leak passing an invalid connection.
"by passing"
> This can happen if the client open a connection and then closes it
"opens"
Acked-by: Christophe Fergeau
_
On Tue, Sep 12, 2017 at 06:42:16AM -0400, Frediano Ziglio wrote:
> In this case you are introducing a regression, current code is
> designed to support this already. I cannot surely say that this
> patch is an improvement.
By that reasoning, any patch which makes some functions static or remove
so
Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
defined this way, otherwise this will break display-channel.c
compilation.
This is different from most AC_DEFINE() preprocessor constants which are
usually either set to 1, or unset.
This commit switches ENABLE_EXTRA_CHECK settin
>
> On Tue, Sep 12, 2017 at 06:20:55AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Sep 12, 2017 at 06:06:29AM -0400, Frediano Ziglio wrote:
> > > > >
> > > > > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > > > > defined this way, otherwise this will break display-
On Tue, Sep 12, 2017 at 06:20:55AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 06:06:29AM -0400, Frediano Ziglio wrote:
> > > >
> > > > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > > > defined this way, otherwise this will break display-channel.c
> > >
>
> On Tue, Sep 12, 2017 at 06:06:29AM -0400, Frediano Ziglio wrote:
> > >
> > > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > > defined this way, otherwise this will break display-channel.c
> > > compilation.
> > > This is different from most AC_DEFINE() preprocessor co
On Tue, Sep 12, 2017 at 06:06:29AM -0400, Frediano Ziglio wrote:
> >
> > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > defined this way, otherwise this will break display-channel.c
> > compilation.
> > This is different from most AC_DEFINE() preprocessor constants which
>
> Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> defined this way, otherwise this will break display-channel.c
> compilation.
> This is different from most AC_DEFINE() preprocessor constants which are
> usually either set to 1, or unset.
>
> This commit switches ENABLE_E
On Tue, Sep 12, 2017 at 05:44:23AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 04:11:57AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> > > > > >
> > > > > > On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Zi
Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
defined this way, otherwise this will break display-channel.c
compilation.
This is different from most AC_DEFINE() preprocessor constants which are
usually either set to 1, or unset.
This commit switches ENABLE_EXTRA_CHECK settin
Command line options are not freed at the end of the program.
These are detected as leaks by leak detector tools like address sanitizer.
Signed-off-by: Frediano Ziglio
---
server/tests/test-gst.c | 36
1 file changed, 24 insertions(+), 12 deletions(-)
Change
>
> On Tue, Sep 12, 2017 at 04:11:57AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> > > > >
> > > > > On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > > > > > > -const EncoderInfo *encoder =
> > > > > > > >
On Tue, Sep 12, 2017 at 03:48:06AM -0400, Frediano Ziglio wrote:
> > > +static inline void
> > > +g_test_assert_expected_messages_internal_no_warnings(const char *domain,
> > > + const char *file,
> > > int
> > > line, const char *func)
> > > +{
On Tue, Sep 12, 2017 at 04:11:57AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > > > > > -const EncoderInfo *encoder = get_encoder_info(encoder_name)
>
> On Tue, Sep 12, 2017 at 04:15:15AM -0400, Frediano Ziglio wrote:
> > >
> > > On Mon, Sep 11, 2017 at 11:15:44AM +0100, Frediano Ziglio wrote:
> > > > Instead of having MarkerPipeItem pointing to a variable on an external
> > > > stuff with the possibility to forget to reset it or having possi
On Tue, Sep 12, 2017 at 04:15:15AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 11, 2017 at 11:15:44AM +0100, Frediano Ziglio wrote:
> > > Instead of having MarkerPipeItem pointing to a variable on an external
> > > stuff with the possibility to forget to reset it or having possibly
> > > dan
>
> On Mon, Sep 11, 2017 at 11:15:44AM +0100, Frediano Ziglio wrote:
> > Instead of having MarkerPipeItem pointing to a variable on an external
> > stuff with the possibility to forget to reset it or having possibly
> > dangling pointers use reference counting to keep the item and
> > mark the ite
>
> On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > > > > -const EncoderInfo *encoder = get_encoder_info(encoder_name);
> > > > > > +const EncoderInfo *encoder = get_encoder_info(encoder_na
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 ca
This helper will be reused by following patch.
Signed-off-by: Frediano Ziglio
Acked-by: Christophe Fergeau
---
server/red-channel-client.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
Changes since v1.
- move SPICE_WATCH_EVENTS_READ_WRITE definition to
On Tue, Sep 12, 2017 at 03:41:15AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > > > -const EncoderInfo *encoder = get_encoder_info(encoder_name);
> > > > > +const EncoderInfo *encoder = get_encoder_info(encoder_name ?
> > > >
On Mon, Sep 11, 2017 at 11:15:44AM +0100, Frediano Ziglio wrote:
> Instead of having MarkerPipeItem pointing to a variable on an external
> stuff with the possibility to forget to reset it or having possibly
> dangling pointers use reference counting to keep the item and
> mark the item when sent.
If a client is unable to complete the TLS handshake phase
reds_init_client_ssl_connection leaked some memory as the stream is not
correctly freed.
This also causes the stream to send the SPICE_CHANNEL_EVENT_DISCONNECTED
event. Otherwise only SPICE_CHANNEL_EVENT_CONNECTED was sent.
Signed-off-by: F
Currently is possible to trigger a leak passing an invalid connection.
This can happen if the client open a connection and then closes it
without writing or reading any data.
Signed-off-by: Frediano Ziglio
---
server/tests/Makefile.am | 1 +
server/tests/test-leaks.c | 12
2 files
On Mon, Sep 11, 2017 at 11:15:43AM +0100, Frediano Ziglio wrote:
> Code tested for the presence of a stream while initializing
> RedChannelClient.
"The code tests for the presence of RedChannelClient::stream while
initializing RedChannelClient"
> However the check was done too late possibly using
>
> 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 ign
>
> 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 som
---
display.js | 27 +--
spiceconn.js | 2 ++
webm.js | 13 +++--
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/display.js b/display.js
index 0868f91..d711ced 100644
--- a/display.js
+++ b/display.js
@@ -543,7 +543,8 @@ SpiceDisplayConn.pro
>
> On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > > -const EncoderInfo *encoder = get_encoder_info(encoder_name);
> > > > +const EncoderInfo *encoder = get_encoder_info(encoder_name ?
> > > > encoder_name : "mjpeg");
> > > > if (!encoder) {
> > > > g_
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
event is received.
Thi
This will be used in other commits.
---
spiceconn.js | 19 +++
1 file changed, 19 insertions(+)
diff --git a/spiceconn.js b/spiceconn.js
index 33e7388..3948ae5 100644
--- a/spiceconn.js
+++ b/spiceconn.js
@@ -243,6 +243,9 @@ SpiceConn.prototype =
else if (this.state == "l
>
> 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
On Tue, Sep 12, 2017 at 03:19:12AM -0400, Frediano Ziglio wrote:
> > > -const EncoderInfo *encoder = get_encoder_info(encoder_name);
> > > +const EncoderInfo *encoder = get_encoder_info(encoder_name ?
> > > encoder_name : "mjpeg");
> > > if (!encoder) {
> > > g_printerr("Encod
On Tue, Sep 12, 2017 at 03:14:22AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 11, 2017 at 11:13:04AM +0100, Frediano Ziglio wrote:
> > > Instead of assuming that the system can safely do unaligned access
> > > to memory use packed structures to allow the compiler generate
> > > best code po
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
--- a/server/tests/test-gst
Pipelines are never freed.
These are detected as leaks by leak detector tools like address sanitizer.
Signed-off-by: Frediano Ziglio
---
server/tests/test-gst.c | 15 +++
1 file changed, 15 insertions(+)
Changes since v1:
- improved commit message.
diff --git a/server/tests/test-gs
Command line options are not freed at the end of the program.
These are detected as leaks by leak detector tools like address sanitizer.
Signed-off-by: Frediano Ziglio
---
server/tests/test-gst.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
Changes since v1:
- imp
>
> 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.
>
>
> >
> > Sign
Instead of assuming that the system can safely do unaligned access
to memory use packed structures to allow the compiler generate
best code possible.
A packed structure tells the compiler to not leave padding inside it
and that the structure can be unaligned so any field can be unaligned
having to
>
> On Mon, Sep 11, 2017 at 11:13:04AM +0100, Frediano Ziglio wrote:
> > Instead of assuming that the system can safely do unaligned access
> > to memory use packed structures to allow the compiler generate
> > best code possible.
>
> It will generate the best code possible, but assuming all acce
65 matches
Mail list logo
