Acked-by: Jonathon Jongsma
On Wed, 2017-09-06 at 18:15 +0100, Frediano Ziglio wrote:
> Cursor resources (basically the shape of it) was retained till
> it was used however it was copied so there were no reason to not
> release
> this resource.
>
> Signed-off-by: Frediano Ziglio
> ---
> server
Hardly any reason for the CommonGraphicsChannel anymore
Acked-by: Jonathon Jongsma
On Wed, 2017-09-06 at 18:15 +0100, Frediano Ziglio wrote:
> Only DisplayChannel uses this property.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/common-graphics-channel.c | 62 ---
Acked-by: Jonathon Jongsma
On Wed, 2017-09-06 at 18:15 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/cursor-channel.c | 5 ++---
> server/cursor-channel.h | 4 ++--
> server/red-worker.c | 2 +-
> 3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --
Not sure that this really needs to be split from the previous patch,
but OK
Acked-by: Jonathon Jongsma
On Wed, 2017-09-06 at 18:15 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/cursor-channel.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
On Wed, 2017-08-30 at 10:36 +0100, Frediano Ziglio wrote:
> You could easily trigger this issue using multiple monitors and
> a modified spice-gtk client with this patch:
>
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1699,6 +1699,7 @@ static gboolean _channel_new(channel_new_t *c)
Acked-by: Jonathon Jongsma
On Wed, 2017-08-30 at 10:36 +0100, Frediano Ziglio wrote:
> You could easily trigger this issue using multiple monitors and
> a modified spice-gtk client with this patch:
>
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1699,6 +1699,7 @@ static gboolean
---
server/dispatcher.h | 102 +---
1 file changed, 82 insertions(+), 20 deletions(-)
diff --git a/server/dispatcher.h b/server/dispatcher.h
index eb93c1358..ed3ecf6bb 100644
--- a/server/dispatcher.h
+++ b/server/dispatcher.h
@@ -35,6 +35,18 @@ typ
This callback was only executed for message types that were registered
with DISPATCHER_ASYNC ack type. However, the async_done handler was
called immediately after the message-specific handler and was called in
the same thread, so the async_done stuff can just as easily be done from
within the mess
So I didn't know you were going to do this and after lunch I basically
implemented the same thing and sent it without noticing this patch :/
Anyway, since then I made a couple of changes and rebased it before the
documentation patch. I'll send an updated patch series.
On Wed, 2017-09-06 at 18:40
And apparently I didn't look at mail for a while and Frediano decided
to do the same thing after our little discussion...
On Wed, 2017-09-06 at 14:03 -0500, Jonathon Jongsma wrote:
> ---
>
> From a previous conversation:
> On Wed, 2017-09-06 at 13:14 -0400, Frediano Ziglio wrote:
> > > [OT: it's
---
From a previous conversation:
On Wed, 2017-09-06 at 13:14 -0400, Frediano Ziglio wrote:
> > [OT: it's a little bit unclear to me why the async_done handler is
> > necessary at all. It is called immediately after the message-
> > specific
> > handler is called, and it is called from the same th
Very quick patch to check Jonathon suggestion, quick tests seems
to indicate that it works
---
server/dispatcher.c | 11 ---
server/dispatcher.h | 16 +---
server/red-worker.c | 52 +---
3 files changed, 26 insertions(+), 53 delet
Signed-off-by: Frediano Ziglio
---
server/cursor-channel.c | 5 ++---
server/cursor-channel.h | 4 ++--
server/red-worker.c | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index fad90ad00..a844100a0 100644
--- a/server/c
Signed-off-by: Frediano Ziglio
---
server/cursor-channel.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index d61cf4808..fad90ad00 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -30,7 +30,6 @@
Cursor resources (basically the shape of it) was retained till
it was used however it was copied so there were no reason to not release
this resource.
Signed-off-by: Frediano Ziglio
---
server/cursor-channel.c | 1 -
server/red-worker.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
Only DisplayChannel uses this property.
Signed-off-by: Frediano Ziglio
---
server/common-graphics-channel.c | 62
server/common-graphics-channel.h | 1 -
server/dcc-send.c| 2 +-
server/dcc.c | 2 +-
server/display-c
>
> On Wed, 2017-09-06 at 12:35 -0400, Frediano Ziglio wrote:
> > >
> > > On Wed, 2017-09-06 at 07:47 -0400, Frediano Ziglio wrote:
> > > > >
> > > > > dispatcher_register_handler() accepts a flag value that
> > > > > determines
> > > > > whether the message type requires an ACK or async_done ha
On Wed, 2017-09-06 at 12:35 -0400, Frediano Ziglio wrote:
> >
> > On Wed, 2017-09-06 at 07:47 -0400, Frediano Ziglio wrote:
> > > >
> > > > dispatcher_register_handler() accepts a flag value that
> > > > determines
> > > > whether the message type requires an ACK or async_done handler.
> > > > Th
>
> On Wed, Sep 06, 2017 at 01:41:21PM +0100, Frediano Ziglio wrote:
> > diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> > index 33f36923a..fda4ff26d 100644
> > --- a/server/red-parse-qxl.c
> > +++ b/server/red-parse-qxl.c
> > @@ -26,6 +26,7 @@
> > #include "red-common.h"
> > #inc
>
> On Wed, 2017-09-06 at 07:47 -0400, Frediano Ziglio wrote:
> > >
> > > dispatcher_register_handler() accepts a flag value that determines
> > > whether the message type requires an ACK or async_done handler.
> > > This
> > > parameter was previously just an int. It is now a typedef'ed enum:
>
The ring is accessed by multiple thread.
Signed-off-by: Frediano Ziglio
---
server/tests/test-display-base.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
Changes since v1:
- moved part to make safe for multiple servers in this patch
diff --git a/server/
There's no need to not compile this feature, it just enable
a parameters which must be passed in order to change test
behaviour.
Signed-off-by: Frediano Ziglio
---
configure.ac | 17 -
server/tests/Makefile.am | 4
server/tests/test-display-base.
The wakeup timer is used by the worker thread and by the
main thread.
Destroying the object before destroying the worker thread
can lead to use after free.
Destroying the worker thread first makes sure we don't race.
This is detected easily when compiling the test with address sanitizer.
Signed-of
This test runs 2 spice server in one program.
Use two different tcp port to be able to connect to both servers.
Signed-off-by: Frediano Ziglio
---
server/tests/test-display-base.c | 8 ++--
server/tests/test-display-base.h | 1 +
server/tests/test-two-servers.c | 2 +-
3 files changed, 8 in
On Wed, 2017-09-06 at 11:23 +0200, Christophe Fergeau wrote:
> On Tue, Sep 05, 2017 at 01:51:05PM -0500, Jonathon Jongsma wrote:
> > void dispatcher_register_async_done_callback(
> > Dispatcher *dispatcher,
> > dispatcher_ha
On Wed, 2017-09-06 at 07:47 -0400, Frediano Ziglio wrote:
> >
> > dispatcher_register_handler() accepts a flag value that determines
> > whether the message type requires an ACK or async_done handler.
> > This
> > parameter was previously just an int. It is now a typedef'ed enum:
> > DispatcherFla
>
> On Wed, Sep 06, 2017 at 11:54:38AM -0400, Frediano Ziglio wrote:
> > >
> > > On Mon, Sep 04, 2017 at 11:57:13AM +0100, Frediano Ziglio wrote:
> > > > For some reasons (documented in cursor_init) the function
> > > > uses 128 bytes more of data causing a reading buffer overflow.
> > >
> > > 1
On Wed, Sep 06, 2017 at 11:54:38AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 04, 2017 at 11:57:13AM +0100, Frediano Ziglio wrote:
> > > For some reasons (documented in cursor_init) the function
> > > uses 128 bytes more of data causing a reading buffer overflow.
> >
> > 128 extra bytes of
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:14AM +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/test-display-base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/server/tests/test-display-base.c
> b/server/tests/te
>
> On Mon, Sep 04, 2017 at 11:57:18AM +0100, Frediano Ziglio wrote:
> > There's no need to not compile this feature, it just enable
> > a parameters which must be passed in order to change test
> > behaviour.
>
> This could be removed from configure.ac too?
>
Was also thinking when I saw the
>
> On Mon, Sep 04, 2017 at 11:57:13AM +0100, Frediano Ziglio wrote:
> > For some reasons (documented in cursor_init) the function
> > uses 128 bytes more of data causing a reading buffer overflow.
>
> 128 extra bytes of data ?
>
> Acked-by: Christophe Fergeau
>
There's this comment/code some
On Mon, Sep 04, 2017 at 11:57:22AM +0100, Frediano Ziglio wrote:
> This test run 2 spice server in one program.
"runs"
> Use two different tcp port to be able to connect to both servers.
> Also fix a race condition getting the command to execute from
> the worker threads.
I would have put the ra
>
> On Mon, Sep 04, 2017 at 11:57:15AM +0100, Frediano Ziglio wrote:
> > Do not use calloc and malloc directly without checking
> > the result. Use instead spice functions to get a nice
> > error in case of allocation failures.
>
> I'd tend to switch to g_new0 and friends rather than spice_xxx...
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:20AM +0100, Frediano Ziglio wrote:
> Update to Glib style to catch warning.
> Initialize properly the structure (invalid) provided.
> Check results of spice_server_init.
> Remove leaks.
> Enable as check.
>
> Signed-off-by: Frediano Zig
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:21AM +0100, Frediano Ziglio wrote:
> Add some check that something happened during creation/destruction.
> Set as running on "make check".
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/Makefile.am | 2 +-
> server/
On Mon, Sep 04, 2017 at 11:57:19AM +0100, Frediano Ziglio wrote:
> Update tests names.
> Remove tetris comments, never available and not planned.
> Update some notes.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/README | 20 ++--
> 1 file changed, 10 insertions(+), 10
On Mon, Sep 04, 2017 at 11:57:18AM +0100, Frediano Ziglio wrote:
> There's no need to not compile this feature, it just enable
> a parameters which must be passed in order to change test
> behaviour.
This could be removed from configure.ac too?
>
> Signed-off-by: Frediano Ziglio
> ---
> serve
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:17AM +0100, Frediano Ziglio wrote:
> This allows to end the loop to end some tests.
> Currently different tests enter the loop but never exit from
> them.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/basic-event-loop.c | 11
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:16AM +0100, Frediano Ziglio wrote:
> In C the sizeof(long) can be different than sizeof(void*),
> use proper type.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/test-display-base.c | 10 +-
> 1 file changed, 5 insert
On Mon, Sep 04, 2017 at 11:57:15AM +0100, Frediano Ziglio wrote:
> Do not use calloc and malloc directly without checking
> the result. Use instead spice functions to get a nice
> error in case of allocation failures.
I'd tend to switch to g_new0 and friends rather than spice_xxx...
Christophe
>
On Mon, Sep 04, 2017 at 11:57:13AM +0100, Frediano Ziglio wrote:
> For some reasons (documented in cursor_init) the function
> uses 128 bytes more of data causing a reading buffer overflow.
128 extra bytes of data ?
Acked-by: Christophe Fergeau
>
> Signed-off-by: Frediano Ziglio
> ---
> Is it
On Mon, Sep 04, 2017 at 11:57:12AM +0100, Frediano Ziglio wrote:
> Timers in spice server are supposed to be used in a single thread
> context. To avoid problems protect the usage from multiple
> thread with a mutex.
"problems, protect"
Acked-by: Christophe Fergeau
> This sometimes caused a cra
I'd add a "when" in the shortlog
On Mon, Sep 04, 2017 at 11:57:11AM +0100, Frediano Ziglio wrote:
> The wakeup timer is used by the worker thread and by the
> main thread.
> Destroying the object before destroying the worker thread
> can lead to use after free.
> Destroying the worker thread befor
Acked-by: Christophe Fergeau
On Mon, Sep 04, 2017 at 11:57:10AM +0100, Frediano Ziglio wrote:
> The ring is accessed by multiple thread.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/test-display-base.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff
On Mon, Sep 04, 2017 at 11:57:09AM +0100, Frediano Ziglio wrote:
> As the indexes are used to compute the index inside an array
> using module operation when a signed value overflows the
"modulo" ... "when a signed value overflows, the modulo becomes
negative, causing a buffer overflow"
Acked-by:
On Wed, Sep 06, 2017 at 01:41:21PM +0100, Frediano Ziglio wrote:
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 33f36923a..fda4ff26d 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -26,6 +26,7 @@
> #include "red-common.h"
> #include "memslot.h"
>
On Wed, Sep 06, 2017 at 10:48:40AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 04, 2017 at 10:47:45AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Fri, Sep 01, 2017 at 10:36:58AM +0100, Frediano Ziglio wrote:
> > > > > These constants are saved into record files so their values should n
>
> On Mon, Sep 04, 2017 at 10:47:45AM -0400, Frediano Ziglio wrote:
> > >
> > > On Fri, Sep 01, 2017 at 10:36:58AM +0100, Frediano Ziglio wrote:
> > > > These constants are saved into record files so their values should not
> > > > be changed in order to be able to use old record files.
> > > >
>
> On Mon, Sep 04, 2017 at 05:09:23PM +0100, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio
> > ---
> > server/char-device.c | 2 +-
> > server/char-device.h | 14 +-
> > server/common-graphics-channel.h | 40 ++--
> > server/cursor-channel
The macros will implement most of the boilerplate needed
to declare an object.
Their usage are similar to GLib G_DECLARE_*_TYPE macros.
Signed-off-by: Frediano Ziglio
---
Changes since v1:
- use SPICE_ prefix instead of GOBJECT_;
- if RED_ prefix is used use this prefix for all GLib
macros.
---
Signed-off-by: Frediano Ziglio
---
server/char-device.h | 14 +-
server/common-graphics-channel.h | 40 ++--
server/cursor-channel-client.h| 18 +--
server/cursor-channel.h | 13 +-
server/dcc.h | 18 +--
server/dispatch
Cursor resources (basically the shape of it) was retained till
it was used however it was copied so there were no reason to not release
this resource.
Signed-off-by: Frediano Ziglio
---
server/cursor-channel.c | 14 --
server/cursor-channel.h | 4 ++--
server/red-par
>
> On Mon, Sep 04, 2017 at 10:39:53AM -0400, Frediano Ziglio wrote:
> > >
> > > On Mon, Sep 04, 2017 at 09:04:03AM -0400, Frediano Ziglio wrote:
> > > > >
> > > > > On Mon, Sep 04, 2017 at 06:06:57AM -0400, Frediano Ziglio wrote:
> > > > > > >
> > > > > > > On Fri, Sep 01, 2017 at 06:19:28AM -
>
> Although dispatcher_send_message() does not allow you to send a message
> type that is invalid for a dispatcher, it still makes sense to verify
> that the type is valid in the receiver. This should only be possible in
> the case of some severe problem such as memory corruption, so if it is
> i
>
> dispatcher_register_handler() accepts a flag value that determines
> whether the message type requires an ACK or async_done handler. This
> parameter was previously just an int. It is now a typedef'ed enum:
> DispatcherFlag. The enum values were accordingly renamed:
>
> DISPATCHER_* -> DISP
>
> How does this relate to G_DECLARE_DERIVABLE_TYPE/G_DECLARE_FINAL_TYPE?
>
Similar but for any type, kind of declare version for G_DEFINE_TYPE.
Why did I choose a GOBJECT_ prefix I don't know...
Frediano
> On Mon, Sep 04, 2017 at 05:09:22PM +0100, Frediano Ziglio wrote:
> > The macro will i
On Mon, Sep 04, 2017 at 10:47:45AM -0400, Frediano Ziglio wrote:
> >
> > On Fri, Sep 01, 2017 at 10:36:58AM +0100, Frediano Ziglio wrote:
> > > These constants are saved into record files so their values should not
> > > be changed in order to be able to use old record files.
> > > Recently 2 diff
On Mon, Sep 04, 2017 at 11:28:45AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Sep 04, 2017 at 10:50:44AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Mon, Sep 04, 2017 at 12:02:05PM +0100, Frediano Ziglio wrote:
> > > > > Now the push is done automatically when a PipeItem is added
> > > >
On Mon, Sep 04, 2017 at 04:02:58PM +0100, Frediano Ziglio wrote:
>
> -static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
> +static RedCursorPipeItem *cursor_pipe_item_new(QXLInstance *qxl,
> RedCursorCmd *cmd)
> {
> -CursorItem *cursor_item;
> +RedCursorPipeItem *it
On Mon, Sep 04, 2017 at 05:09:23PM +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/char-device.c | 2 +-
> server/char-device.h | 14 +-
> server/common-graphics-channel.h | 40 ++--
> server/cursor-channel-client.h| 18
How does this relate to G_DECLARE_DERIVABLE_TYPE/G_DECLARE_FINAL_TYPE?
On Mon, Sep 04, 2017 at 05:09:22PM +0100, Frediano Ziglio wrote:
> The macro will implement most of the boilerplate needed
> to declare an object.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-common.h | 34 +
On Mon, Sep 04, 2017 at 05:28:29PM +0100, Frediano Ziglio wrote:
> Is possible to have a leak processing update commands if
> the update command is synchronous and the rectangle list
> is empty. Note that Qemu always pass an empty list.
>
> If the list is empty display_channel_update fill the list
Acked-by: Christophe Fergeau
On Tue, Sep 05, 2017 at 01:51:04PM -0500, Jonathon Jongsma wrote:
> dispatcher_register_handler() accepts a flag value that determines
> whether the message type requires an ACK or async_done handler. This
> parameter was previously just an int. It is now a typedef'e
On Tue, Sep 05, 2017 at 01:51:05PM -0500, Jonathon Jongsma wrote:
> void dispatcher_register_async_done_callback(
> Dispatcher *dispatcher,
> dispatcher_handle_async_done handler);
>
> -/*
> - * Hack to allow red_record to
On 09/05/2017 09:51 PM, Jonathon Jongsma wrote:
---
server/dispatcher.h | 140 +++-
1 file changed, 116 insertions(+), 24 deletions(-)
diff --git a/server/dispatcher.h b/server/dispatcher.h
index 862fc46b9..058032dbb 100644
--- a/server/dispatch
---
main.js | 15 +++
spice.html | 5 +
spice_auto.html | 5 +
3 files changed, 25 insertions(+)
diff --git a/main.js b/main.js
index 0237f0e..447177b 100644
--- a/main.js
+++ b/main.js
@@ -385,20 +385,25 @@ SpiceMainConn.prototype.handle_file_xfer_status =
fun
---
enums.js| 12 ++--
main.js | 16 +++-
spicemsg.js | 4
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/enums.js b/enums.js
index d5a9003..7154595 100644
--- a/enums.js
+++ b/enums.js
@@ -369,12 +369,20 @@ var VD_AGENT_CAP_MOUSE_STATE
---
cursor.js| 21 +++--
png.js | 2 +-
spice.html | 1 +
spice_auto.html | 1 +
thirdparty/monocursor.js | 79
5 files changed, 94 insertions(+), 10 deletions(-)
create mode 10064
68 matches
Mail list logo
