Re: [Spice-devel] [spice-gtk v2] usbredir: fix deadlock on error

On Tue, 2017-06-20 at 13:10 +0200, Victor Toso wrote: > From: leaboy > > The current code deadlocks when a USB error occurs, releasing the > channel lock before the idle is called fixes this problem. > > More specifically when an error occurs, we queue a call to device_error

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

> > > On 25 Jul 2017, at 19:37, Christophe Fergeau wrote: > > > > On Tue, Jul 25, 2017 at 02:26:36PM +0200, Christophe de Dinechin wrote: > > As long as contributor keep pinging or resending his series, this is > > already the case. > > As Frediano said

Re: [Spice-devel] [vdagent-win PATCH 1/3] Make VDLog::printf static

On Wed, Jul 26, 2017 at 11:03:40AM +0100, Frediano Ziglio wrote: > VDLog::printf is called only using a VDLog pointer. > Being this class a singleton there could only one instance of > the class so the pointer must be the single instance. > This simplify LOG macros not having to choose between

Re: [Spice-devel] [vdagent-win PATCH] Prevent possible future buffer overflow

> On 26 Jul 2017, at 09:21, Frediano Ziglio wrote: > > event_type should come only with specific values but > this in theory can change in the future. > To prevent overflows (just for logging) check value size > against lookup array. > > Signed-off-by: Frediano Ziglio

Re: [Spice-devel] [vdagent-win PATCH v12 3/6] Write code to decode PNG format

> > On 25 Jul 2017, at 20:01, Frediano Ziglio < fzig...@redhat.com > wrote: > > > > On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote: > > > > > > > +size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data, > > > > > > > > > > size_t size) > > > > > > > > > >

Re: [Spice-devel] [PATCH spice-gtk] Use designated struct initializer

On Fri, 2017-07-07 at 16:26 +0200, Pavel Grunt wrote: > Silence -Wmissing-field-initializers warnings. Forgot to mention, it happens when building tests. My gcc --version: gcc (GCC) 7.1.1 20170622 (Red Hat 7.1.1-3) But I'm not sure it is specific to gcc version, a similar fix has been replied

Re: [Spice-devel] [vdagent-win PATCH 3/3] Add a test for logging functions

On Wed, Jul 26, 2017 at 07:01:05AM -0400, Frediano Ziglio wrote: > > > > On Wed, Jul 26, 2017 at 11:03:42AM +0100, Frediano Ziglio wrote: > > > Signed-off-by: Frediano Ziglio > > > --- > > > Makefile.am | 13 + > > > common/test-log.cpp | 53 > > >

Re: [Spice-devel] [PATCH spice-gtk] Use designated struct initializer

Hey, On Wed, Jul 26, 2017 at 11:12:25AM +0200, Pavel Grunt wrote: > On Fri, 2017-07-07 at 16:26 +0200, Pavel Grunt wrote: > > Silence -Wmissing-field-initializers warnings. > > Forgot to mention, it happens when building tests. My gcc --version: > gcc (GCC) 7.1.1 20170622 (Red Hat 7.1.1-3) I'm

Re: [Spice-devel] [vdagent-win PATCH 3/3] Add a test for logging functions

On Wed, Jul 26, 2017 at 11:03:42AM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > Makefile.am | 13 + > common/test-log.cpp | 53 > + > test-log| 3 +++ > 3 files

Re: [Spice-devel] [vdagent-win PATCH 2/3] Expand PRINT_LINE macros

Acked-by: Christophe Fergeau On Wed, Jul 26, 2017 at 11:03:41AM +0100, Frediano Ziglio wrote: > This complicated macro was used as an helper for LOG macro > which expand PRINT_LINE macro twice. Now that LOG expand PRINT_LINE > only once there's no need to have this macro

Re: [Spice-devel] [PATCH spice-gtk] Use designated struct initializer

On Fri, Jul 07, 2017 at 04:26:33PM +0200, Pavel Grunt wrote: > Silence -Wmissing-field-initializers warnings. This doesn't make sense for many of these cases you've changed > diff --git a/src/channel-main.c b/src/channel-main.c > index 4edd575..104b18a 100644 > --- a/src/channel-main.c > +++

Re: [Spice-devel] [spice-gtk v2] usbredir: fix deadlock on error

Hi, On Wed, Jul 26, 2017 at 02:48:41PM +0200, Pavel Grunt wrote: > On Tue, 2017-06-20 at 13:10 +0200, Victor Toso wrote: > > From: leaboy > > > > The current code deadlocks when a USB error occurs, releasing the > > channel lock before the idle is called fixes this problem. >

[Spice-devel] [vdagent-win PATCH] Avoid using LTO by default during compilation

LTO option allows to have smaller executable stripping unneeded part of code. These options were added by a095f4806e ("build-sys: statically build agent"). The gain of using these options currently are quite small, from a small test: $ ll *.exe -rwxrwxr-x. 1 freddy freddy 506880 Jul 26 11:26

[Spice-devel] [vdagent-win PATCH v2 3/3] Add a test for logging functions

Signed-off-by: Frediano Ziglio --- Makefile.am | 13 common/test-log.cpp | 57 + test-log| 3 +++ 3 files changed, 73 insertions(+) create mode 100644 common/test-log.cpp create mode

[Spice-devel] [vdagent-win PATCH v2 1/3] Make VDLog::printf static

VDLog::printf is called only using a VDLog pointer. Since this class is a singleton, there can only be one instance of it, so LOG() does not need really need to deal with it, VDLog::printf can do that instead. This simplify LOG macros not having to choose between printf and VDLog::printf. The

[Spice-devel] [vdagent-win PATCH v2 2/3] Expand PRINT_LINE macros

This complicated macro was used as an helper for LOG macro which expand PRINT_LINE macro twice. Now that LOG expand PRINT_LINE only once there's no need to have this macro which make logging code more complicated. Signed-off-by: Frediano Ziglio Acked-by: Christophe Fergeau

Re: [Spice-devel] [vdagent-win PATCH 3/3] Add a test for logging functions

> > On Wed, Jul 26, 2017 at 11:03:42AM +0100, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > Makefile.am | 13 + > > common/test-log.cpp | 53 > > + > > test-log| 3

Re: [Spice-devel] [vdagent-win PATCH] Avoid using LTO by default during compilation

On Wed, Jul 26, 2017 at 11:38:08AM +0100, Frediano Ziglio wrote: > LTO option allows to have smaller executable stripping > unneeded part of code. > These options were added by a095f4806e ("build-sys: statically > build agent"). > The gain of using these options currently are quite small, > from a

Re: [Spice-devel] [vdagent-win PATCH v13 4/6] Support encoding PNG images

Acked-by: Christophe Fergeau On Tue, Jul 25, 2017 at 06:33:34PM +0100, Frediano Ziglio wrote: > -uint8_t *PngCoder::from_bitmap(const BITMAPINFO& info, const void *bits, > long ) > +uint8_t *PngCoder::from_bitmap(const BITMAPINFO& bmp_info, const void *bits, > long )

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

Hi - Original Message - > Now, any objection to > > 1. Recommending that we use git URLs in patches? If that may help, but as Christophe said, this may be overkill for small series. Let's not make it a rule. > 2. Having a shared location for branches under review? This is really

Re: [Spice-devel] [vdagent-win PATCH v13 2/6] Initial rewrite of image conversion code

Acked-by: Christophe Fergeau On Tue, Jul 25, 2017 at 06:33:32PM +0100, Frediano Ziglio wrote: > Remove CxImage linking. > Support Windows BMP format. > > Signed-off-by: Frediano Ziglio > Acked-by: Christophe Fergeau > --- >

Re: [Spice-devel] [vdagent-win PATCH v13 6/6] spec: run tests during RPM build if possible

On Tue, Jul 25, 2017 at 06:33:36PM +0100, Frediano Ziglio wrote: > Currently to fully run checks we need Wine for both 32 and 64 bit. > Some distros (like RHEL 7) don't seem to allow installing > both 32 and 64 bit versions so turn on checks based on distro > versions. > > Signed-off-by: Frediano

Re: [Spice-devel] [vdagent-win PATCH v13 5/6] Add test for PNG files

Acked-by: Christophe Fergeau On Tue, Jul 25, 2017 at 06:33:35PM +0100, Frediano Ziglio wrote: > Test various image and formats. > The idea is to decode and encode again an image and > check for differences. ImageMagick is used to create > some test image and compare

Re: [Spice-devel] [vdagent-win PATCH v13 3/6] Write code to decode PNG format

Acked-by: Christophe Fergeau On Tue, Jul 25, 2017 at 06:33:33PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > Makefile.am | 6 +- > configure.ac | 3 + > vdagent/image.cpp| 8 +- >

Re: [Spice-devel] [vdagent-win PATCH v12 3/6] Write code to decode PNG format

> On 25 Jul 2017, at 20:01, Frediano Ziglio wrote: > >> >> On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote: >>> +size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data, >>> size_t size) >>> +{ >> [...] >>> +const unsigned int width =

Re: [Spice-devel] [vdagent-win PATCH v12 3/6] Write code to decode PNG format

> On 26 Jul 2017, at 09:58, Frediano Ziglio wrote: > > > On 25 Jul 2017, at 20:01, Frediano Ziglio > wrote: > > > On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote: > +size_t PngCoder::convert_to_dib(uint8_t

[Spice-devel] [vdagent-win PATCH 1/3] Make VDLog::printf static

VDLog::printf is called only using a VDLog pointer. Being this class a singleton there could only one instance of the class so the pointer must be the single instance. This simplify LOG macros not having to choose between printf and VDLog::printf. The manual GNU __attribute__ is used as the format

[Spice-devel] [vdagent-win PATCH 3/3] Add a test for logging functions

Signed-off-by: Frediano Ziglio --- Makefile.am | 13 + common/test-log.cpp | 53 + test-log| 3 +++ 3 files changed, 69 insertions(+) create mode 100644 common/test-log.cpp create mode

[Spice-devel] [vdagent-win PATCH 2/3] Expand PRINT_LINE macros

This complicated macro was used as an helper for LOG macro which expand PRINT_LINE macro twice. Now that LOG expand PRINT_LINE only once there's no need to have this macro which make logging code more complicated. Signed-off-by: Frediano Ziglio --- common/vdlog.h | 9

[Spice-devel] [vdagent-win PATCH] Prevent possible future buffer overflow

event_type should come only with specific values but this in theory can change in the future. To prevent overflows (just for logging) check value size against lookup array. Signed-off-by: Frediano Ziglio --- vdservice/vdservice.cpp | 3 ++- 1 file changed, 2 insertions(+), 1

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

On Tue, Jul 25, 2017 at 01:55:11PM -0400, Frediano Ziglio wrote: > > > > On Tue, Jul 25, 2017 at 07:09:22AM -0400, Frediano Ziglio wrote: > > > > > > > > > >>> I see several benefits to doing this: > > > > > >>> > > > > > >>> 1. We always know exactly which component and branch is being > > > >

Re: [Spice-devel] [vdagent-win PATCH v12 3/6] Write code to decode PNG format

> > On 26 Jul 2017, at 09:58, Frediano Ziglio < fzig...@redhat.com > wrote: > > > > > On 25 Jul 2017, at 20:01, Frediano Ziglio < fzig...@redhat.com > wrote: > > > > > > > > > > > On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote: > > > > > > > > > > > > > > > > +size_t

Re: [Spice-devel] [vdagent-win PATCH v13 6/6] spec: run tests during RPM build if possible

> > On Tue, Jul 25, 2017 at 06:33:36PM +0100, Frediano Ziglio wrote: > > Currently to fully run checks we need Wine for both 32 and 64 bit. > > Some distros (like RHEL 7) don't seem to allow installing > > both 32 and 64 bit versions so turn on checks based on distro > > versions. > > > >

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

> On 25 Jul 2017, at 19:37, Christophe Fergeau wrote: > > On Tue, Jul 25, 2017 at 02:26:36PM +0200, Christophe de Dinechin wrote: > As long as contributor keep pinging or resending his series, this is > already the case. As Frediano said at the beginning

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

On Wed, Jul 26, 2017 at 09:18:37AM +0200, Christophe de Dinechin wrote: > Now, any objection to > > 1. Recommending that we use git URLs in patches? We can emphasize this, but this has been done in the past, and anyone is free to do it anyway if they want ;) For what it's worth, for small series

Re: [Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

On Tue, Jul 25, 2017 at 02:23:28PM -0400, Frediano Ziglio wrote: > > > > On Tue, Jul 25, 2017 at 07:20:23PM +0200, Christophe Fergeau wrote: > > > On Tue, Jul 25, 2017 at 07:09:22AM -0400, Frediano Ziglio wrote: > > > > > > > > > > > >>> I see several benefits to doing this: > > > > > > >>> > >

Re: [Spice-devel] [vdagent-win PATCH v13 6/6] spec: run tests during RPM build if possible

On Wed, Jul 26, 2017 at 06:04:48AM -0400, Frediano Ziglio wrote: > > > > On Tue, Jul 25, 2017 at 06:33:36PM +0100, Frediano Ziglio wrote: > > > Currently to fully run checks we need Wine for both 32 and 64 bit. > > > Some distros (like RHEL 7) don't seem to allow installing > > > both 32 and 64

[Spice-devel] [vdagent-win PATCH v3] Add a test for logging functions

Signed-off-by: Frediano Ziglio --- Makefile.am | 13 + common/test-log.cpp | 55 + test-log| 3 +++ 3 files changed, 71 insertions(+) create mode 100644 common/test-log.cpp create mode

Re: [Spice-devel] [vdagent-win PATCH v3] Add a test for logging functions

Acked-by: Christophe Fergeau On Wed, Jul 26, 2017 at 03:48:25PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > Makefile.am | 13 + > common/test-log.cpp | 55 >

Re: [Spice-devel] [vdagent-win PATCH] Do not use dash in rpm version

Hi, Cool, looks good to me I can confirm that thanks to this patch is possible to do things like: 1. make dist 2. md5sum vdagent-win-0.8.0.34-7b15.tar.xz > sources 3. fedpkg srpm And for instance use the srpm to create a build in koji/brew/copr On Wed, 2017-07-26 at 16:03 +0100,

[Spice-devel] [vdagent-win PATCH] Do not use dash in rpm version

RPM does not allow dash in version string. Remove everything after the dash (which should be the "dirty" git version). This make easier to run "make dist" followed by rpmbuild. Signed-off-by: Frediano Ziglio --- configure.ac| 2 ++ mingw-spice-vdagent.spec.in

Re: [Spice-devel] [vdagent-win PATCH v2 1/3] Make VDLog::printf static

On Wed, Jul 26, 2017 at 12:49:15PM +0100, Frediano Ziglio wrote: > VDLog::printf is called only using a VDLog pointer. > Since this class is a singleton, there can only be one instance of it, > so LOG() does not need really need to deal with it, VDLog::printf can > do that instead. "need really

Re: [Spice-devel] [vdagent-win PATCH v2 3/3] Add a test for logging functions

Acked-by: Christophe Fergeau On Wed, Jul 26, 2017 at 12:49:17PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > Makefile.am | 13 > common/test-log.cpp | 57 >