Re: [Spice-devel] [PATCH spice-common] recorder: Update

[Eduardo Lima (Etrunko)]

On 3/29/19 6:47 AM, Frediano Ziglio wrote:
> Pull some fixes and features.
> One of the feature is the support for @output setting to redirect
> log output.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  common/recorder | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/recorder b/common/recorder
> index 03eb4b6..10b1787 16
> --- a/common/recorder
> +++ b/common/recorder
> @@ -1 +1 @@
> -Subproject commit 03eb4b6ef7aa90645a7395ea8bea55ebf33d6632
> +Subproject commit 10b178762e1d66d82bdde754385a6a608a730607
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v3] log: Let gcc know about the logging macros which abort

[Christophe Fergeau]

This commit adds a SPICE_UNREACHABLE macro (courtesy of Frediano)
so that gcc does not think that code control can go past
spice_return_{val_,}if_fail(), spice_critical() and spice_error()

This avoids this kind of warnings:

fallthrough.c:

 #include "log.h"

int main(int argc, char **argv)
{
switch(argc) {
case 1:
spice_critical("foo");
   default:
return 0;
}
}

$ gcc  -c$(pkg-config --cflags --libs glib-2.0 spice-protocol)
   -I common   -Wimplicit-fallthrough=5 ./fallthrough.c
In file included from ./fallthrough.c:1:
./fallthrough.c: In function 'main':
common/log.h:73:5: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   73 | spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" 
format, ## __VA_ARGS__); \
  | 
^~
./fallthrough.c:8:25: note: in expansion of macro 'spice_critical'
8 | spice_critical("foo");
  | ^~
./fallthrough.c:9:17: note: here
9 | default:
  | ^~~

Signed-off-by: Christophe Fergeau 
---
Maybe a bit too much for a single commit, I can split :)

 common/log.h| 8 ++--
 common/macros.h | 8 
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/common/log.h b/common/log.h
index 7c67e7a..201c87a 100644
--- a/common/log.h
+++ b/common/log.h
@@ -39,16 +39,18 @@ void spice_log(GLogLevelFlags log_level,
const char *format,
...) G_GNUC_PRINTF(4, 5);
 
+/* FIXME: name is misleading, this aborts.. */
 #define spice_return_if_fail(x) G_STMT_START {  \
 if G_LIKELY(x) { } else {   \
-spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition 
`%s' failed", #x); \
+spice_critical("condition `%s' failed", #x);\
 return; \
 }   \
 } G_STMT_END
 
+/* FIXME: name is misleading, this aborts.. */
 #define spice_return_val_if_fail(x, val) G_STMT_START { \
 if G_LIKELY(x) { } else {   \
-spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "condition 
`%s' failed", #x); \
+spice_critical("condition `%s' failed", #x);\
 return (val);   \
 }   \
 } G_STMT_END
@@ -71,10 +73,12 @@ void spice_log(GLogLevelFlags log_level,
 
 #define spice_critical(format, ...) G_STMT_START {  \
 spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format, ## 
__VA_ARGS__); \
+SPICE_UNREACHABLE; 
 \
 } G_STMT_END
 
 #define spice_error(format, ...) G_STMT_START { \
 spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, "" format, ## 
__VA_ARGS__); \
+SPICE_UNREACHABLE; 
  \
 } G_STMT_END
 
 #define spice_warn_if_fail(x) G_STMT_START {\
diff --git a/common/macros.h b/common/macros.h
index 2f24ada..92cd82c 100644
--- a/common/macros.h
+++ b/common/macros.h
@@ -55,4 +55,12 @@
 
 #define SPICE_VERIFY(cond) verify_expr(cond, (void)1)
 
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
+#define SPICE_UNREACHABLE __builtin_unreachable()
+#elif defined(_MSC_VER)
+#define SPICE_UNREACHABLE __assume(0)
+#else
+#define SPICE_UNREACHABLE for(;;) continue
+#endif
+
 #endif // H_SPICE_COMMON_MACROS
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol] protocol: Generate enums.h again to remove old protocol definitions

[Christophe Fergeau]

On Wed, Mar 27, 2019 at 04:24:02AM -0400, Frediano Ziglio wrote:
> ping

Acked-by: Christophe Fergeau 
though imo this should be reflected in version numbering ;)

> 
> > > 
> > > This is breaking spice-protocol API even if this should not impact
> > > current code. Do we want to advertise this somehow? Switch to
> > > spice-protocol 0.14.x?
> > > 
> > 
> > Fine for me both 0.14.0 or 0.12.16 (the future, current HEAD version)
> > 
> > > On Fri, Feb 22, 2019 at 12:16:52PM +, Frediano Ziglio wrote:
> > > > spice.proto was updated to remove really old definitions,
> > > > update enums.h accordingly.
> > > > 
> > > > Signed-off-by: Frediano Ziglio 
> > > > ---
> > > >  spice/enums.h | 57 ---
> > > >  1 file changed, 57 deletions(-)
> > > > 
> > > > diff --git a/spice/enums.h b/spice/enums.h
> > > > index 34e84c3..172cc4d 100644
> > > > --- a/spice/enums.h
> > > > +++ b/spice/enums.h
> > > > @@ -119,21 +119,6 @@ typedef enum SpiceMouseMode {
> > > >  SPICE_MOUSE_MODE_MASK = 0x3
> > > >  } SpiceMouseMode;
> > > >  
> > > > -typedef enum SpicePubkeyType {
> > > > -SPICE_PUBKEY_TYPE_INVALID,
> > > > -SPICE_PUBKEY_TYPE_RSA,
> > > > -SPICE_PUBKEY_TYPE_RSA2,
> > > > -SPICE_PUBKEY_TYPE_DSA,
> > > > -SPICE_PUBKEY_TYPE_DSA1,
> > > > -SPICE_PUBKEY_TYPE_DSA2,
> > > > -SPICE_PUBKEY_TYPE_DSA3,
> > > > -SPICE_PUBKEY_TYPE_DSA4,
> > > > -SPICE_PUBKEY_TYPE_DH,
> > > > -SPICE_PUBKEY_TYPE_EC,
> > > > -
> > > > -SPICE_PUBKEY_TYPE_ENUM_END
> > > > -} SpicePubkeyType;
> > > > -
> > > >  typedef enum SpiceDataCompressionType {
> > > >  SPICE_DATA_COMPRESSION_TYPE_NONE,
> > > >  SPICE_DATA_COMPRESSION_TYPE_LZ4,
> > > > @@ -397,21 +382,6 @@ typedef enum SpiceAudioFmt {
> > > >  SPICE_AUDIO_FMT_ENUM_END
> > > >  } SpiceAudioFmt;
> > > >  
> > > > -typedef enum SpiceTunnelServiceType {
> > > > -SPICE_TUNNEL_SERVICE_TYPE_INVALID,
> > > > -SPICE_TUNNEL_SERVICE_TYPE_GENERIC,
> > > > -SPICE_TUNNEL_SERVICE_TYPE_IPP,
> > > > -
> > > > -SPICE_TUNNEL_SERVICE_TYPE_ENUM_END
> > > > -} SpiceTunnelServiceType;
> > > > -
> > > > -typedef enum SpiceTunnelIpType {
> > > > -SPICE_TUNNEL_IP_TYPE_INVALID,
> > > > -SPICE_TUNNEL_IP_TYPE_IPv4,
> > > > -
> > > > -SPICE_TUNNEL_IP_TYPE_ENUM_END
> > > > -} SpiceTunnelIpType;
> > > > -
> > > >  typedef enum SpiceVscMessageType {
> > > >  SPICE_VSC_MESSAGE_TYPE_Init = 1,
> > > >  SPICE_VSC_MESSAGE_TYPE_Error,
> > > > @@ -613,33 +583,6 @@ enum {
> > > >  SPICE_MSGC_END_RECORD
> > > >  };
> > > >  
> > > > -enum {
> > > > -SPICE_MSG_TUNNEL_INIT = 101,
> > > > -SPICE_MSG_TUNNEL_SERVICE_IP_MAP,
> > > > -SPICE_MSG_TUNNEL_SOCKET_OPEN,
> > > > -SPICE_MSG_TUNNEL_SOCKET_FIN,
> > > > -SPICE_MSG_TUNNEL_SOCKET_CLOSE,
> > > > -SPICE_MSG_TUNNEL_SOCKET_DATA,
> > > > -SPICE_MSG_TUNNEL_SOCKET_CLOSED_ACK,
> > > > -SPICE_MSG_TUNNEL_SOCKET_TOKEN,
> > > > -
> > > > -SPICE_MSG_END_TUNNEL
> > > > -};
> > > > -
> > > > -enum {
> > > > -SPICE_MSGC_TUNNEL_SERVICE_ADD = 101,
> > > > -SPICE_MSGC_TUNNEL_SERVICE_REMOVE,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_ACK,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_NACK,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_FIN,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED_ACK,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_DATA,
> > > > -SPICE_MSGC_TUNNEL_SOCKET_TOKEN,
> > > > -
> > > > -SPICE_MSGC_END_TUNNEL
> > > > -};
> > > > -
> > > >  enum {
> > > >  SPICE_MSG_SMARTCARD_DATA = 101,
> > > >  
> > 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] char-device: Make RedClient an opaque structure again

[Christophe Fergeau]

On Tue, Mar 12, 2019 at 05:58:40AM -0400, Frediano Ziglio wrote:
> ping

I guess I would reword this as "char-device: Don't use RedClient API"
RedCharDevice only use red_client_get_server() once, we can store a
Reds* in RedCharDeviceClient instead. This will make it possible to turn
the RedClient reference into a generic pointer/handle in a later commit,
which will be useful if we want to split the flow control part of
char-device in a more specialised file

(not sure the last part is accurate regarding your goal ;)

Dunno how that sounds?

Christophe

> 
> > 
> > RedClient was an opaque structure for RedCharDevice.
> > It started to be used when RedsState started to contain all
> > the global state.
> > Make it opaque again.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  server/char-device.c | 16 +++-
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/server/char-device.c b/server/char-device.c
> > index 040b91147..465c1a125 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -22,8 +22,8 @@
> >  
> >  #include 
> >  #include 
> > +
> >  #include "char-device.h"
> > -#include "red-client.h"
> >  #include "reds.h"
> >  #include "glib-compat.h"
> >  
> > @@ -703,11 +703,10 @@ void red_char_device_destroy(RedCharDevice *char_dev)
> >  g_object_unref(char_dev);
> >  }
> >  
> > -static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
> > -   int do_flow_control,
> > -   uint32_t
> > max_send_queue_size,
> > -   uint32_t
> > num_client_tokens,
> > -   uint32_t
> > num_send_tokens)
> > +static RedCharDeviceClient *
> > +red_char_device_client_new(RedsState *reds, RedClient *client,
> > +   int do_flow_control, uint32_t
> > max_send_queue_size,
> > +   uint32_t num_client_tokens, uint32_t
> > num_send_tokens)
> >  {
> >  RedCharDeviceClient *dev_client;
> >  
> > @@ -717,8 +716,6 @@ static RedCharDeviceClient
> > *red_char_device_client_new(RedClient *client,
> >  dev_client->max_send_queue_size = max_send_queue_size;
> >  dev_client->do_flow_control = do_flow_control;
> >  if (do_flow_control) {
> > -RedsState *reds = red_client_get_server(client);
> > -
> >  dev_client->wait_for_tokens_timer =
> >  reds_core_timer_add(reds, 
> > device_client_wait_for_tokens_timeout,
> >  dev_client);
> > @@ -757,7 +754,8 @@ bool red_char_device_client_add(RedCharDevice *dev,
> >  dev->priv->wait_for_migrate_data = wait_for_migrate_data;
> >  
> >  spice_debug("char device %p, client %p", dev, client);
> > -dev_client = red_char_device_client_new(client, do_flow_control,
> > +dev_client = red_char_device_client_new(dev->priv->reds, client,
> > +do_flow_control,
> >  max_send_queue_size,
> >  num_client_tokens,
> >  num_send_tokens);
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Two monitors Windows

[Christophe Fergeau]

Hey,

Your VM configuration does not seem to have a spicevmc channel device,
so I assume spice-vdagent is not running in the guest? This is required
for proper mouse behaviour in multiscreen setups.

Christophe

On Thu, Mar 28, 2019 at 10:59:16AM +0100, Mathias Egekvist wrote:
> Hi Spice Developer(s), 
> 
> Not sure this email should go to you, but I am not sure where else to go.
> 
> Recently started using Spice with libvirt and found great performance. 
> I have started a Windows 10 guest vm and it is working fine with 1 monitor. 
> I saw on your website  
> that to add 2 monitors I had to add an extra QXL device and it worked.
> Suddenly the mouse stopped working correctly though and I have been unable to 
> find a solution. When I remove the extra video tag everything works fine 
> again. 
> 
> Is it you I need to ask or someone else?
> If it is a general problem and I can help, please let me know.  
> I'll add all my information below.
> 
> 
> Best regards,
> 
> Mathias Egekvist
> 
> 
> Problem in details:
> The mouse either "glues" it self to the furthest left-side or only move in Y 
> and X-axis as in only one at a time. Sometimes the mouse move normally 
> though, but the problem which persists is every time I click it goes to the 
> top left corner and registers the click there. 
> 
> Setup Details:
> System is Arch Linux and window manager is dwm. 
> VM is Windows 10 running on KVM through libvirt, seen through virt-viewer 
> (have also tried remote-viewer).
> I have the EvTouch USB Graphics Tablet added together with a mouse and 
> keyboard.
> I have two QXL displays added, where I have tried adjusting the different ram 
> options, currently ram & vram 65536 and vgamem 16384. 
> Screen/graphics is Spice server, listen type have tried both none and 
> address. Tried with address 'All Interfaces' with port and tls and opengl on 
> and off. 
> XML you can see here: 
> https://unix.stackexchange.com/questions/507725/cursor-jumps-to-left-corner-windows-10-vm-kvm
>  
> 
>   

> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 2/2] docs: Add some notes on event scheduling and threading

[Christophe Fergeau]

On Thu, Mar 28, 2019 at 10:27:46AM -0400, Frediano Ziglio wrote:
> > On Thu, Mar 28, 2019 at 04:25:31AM -0400, Frediano Ziglio wrote:
> > > > 
> > > > On Mon, Mar 11, 2019 at 02:03:33PM +, Frediano Ziglio wrote:
> > > > > Signed-off-by: Frediano Ziglio 
> > > > > ---
> > > > >  docs/spice_threading_model.txt | 8 
> > > > >  1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/docs/spice_threading_model.txt
> > > > > b/docs/spice_threading_model.txt
> > > > > index 9351141c8..25a3a030c 100644
> > > > > --- a/docs/spice_threading_model.txt
> > > > > +++ b/docs/spice_threading_model.txt
> > > > > @@ -39,6 +39,14 @@ connect, disconnect and migrate. Connect and 
> > > > > migrate
> > > > > are
> > > > > asynchronous (the job
> > > > >  is done while the current thread is doing something else) while
> > > > >  disconnect
> > > > >  is
> > > > >  synchronous (the main thread will wait for termination).
> > > > >  
> > > > > +One aspect to take into consideration is the event scheduling. SPICE
> > > > > uses
> > > > > some
> > > > > +`SpiceCoreInterface` to handle events. As the events will be handled
> > > > > from
> > > > > a
> > > > > +thread based on the core interface you have to use the correct core.
> > > > > Each
> > > > > +channel has an associated core interface which can be retrieved using
> > > > > +`red_channel_get_core_interface`. There's also a main core interface
> > > > > you
> > > > > can get
> > > > > +using `reds_get_core_interface`. `reds_core_timer_*` and
> > > > > `reds_core_watch_*`
> > > > > +functions use the main core interface.
> > > > 
> > > > Do we need a few words as to when to use the main core interface?
> > > > Apart from this, looks good to me.
> > > > 
> > > > Christophe
> > > > 
> > > 
> > > It sounds a nice idea.
> > > 
> > > But honestly I cannot came with an easy rule beside "If code runs on
> > > main thread like Qemu character devices or everything not running in
> > > a channel you can use the main core interface."
> > 
> > Yes, something like your rule would work "Code running in the QEMU
> > thread should use the main core interface. Code running in the cursor or
> > display channel (through RedWorker) should use xxx interface.. Code
> > running in other channels should use yyy. Be aware that a channel's
> 
> IMHO all code running in channels should use channel core, no matter
> if they run on main or not (in the past I adjusted this).
> Note that cursor channel code can run in both main and not main
> for instance, not all cursor channels runs under RedWorker.
> I think it's easier to avoid too much exceptions.
> 
> > ClientCbs run in a different thread context than the rest of the
> > channel" (though the last sentence may no longer be accurate with the
> > work you are doing in that area).
> 
> ClientCbs will be removed, one less thing to know, and new callbacks/vfunc
> will work on the channel thread so not different from other channel code.
> 
> > 
> > Christophe
> > 
> 
> Taking into account that ClientCbs part will be soon (I hope) obsolete
> and that part of "channel core for channel core" part is partially there
> I would update to
> 
> +One aspect to take into consideration is the event scheduling. SPICE uses 
> some
> +`SpiceCoreInterface` to handle events. As the events will be handled from a
> +thread based on the core interface you have to use the correct core. Each
> +channel has an associated core interface which can be retrieved using
> +`red_channel_get_core_interface`. There's also a main core interface you can 
> get
> +using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*`
> +functions use the main core interface.


> +Even though multiple channel types run in the main thread and so could use
> +directly the main code interface, for coherence, rule simplicity and to 
> allows
> +the code to be moved in a separate thread easily if needed, it's advisable to
> +use the channel core interface (that will be the main core interface in this
> +case).

I'm not sure if this paragraph makes things clearer or more confusing
(because too many details). Maybe remove it?

> +Currently character devices and not channel code runs in the main thread.

Is this here to mean character device code should be using reds_core_*?

Overall looks good to me.

> OT: I though QEMU moved to GLib to handle events (so the main core interface
> was using GLib) but for performance reasons epoll or other implementations
> are used (that's the reason for SOCKET limitation for Windows).

Ah ok, I assumed epoll was there for historical reasons, not because
of performance. 

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] red-channel: Small comment on "core" field

[Christophe Fergeau]

For the series,

Acked-by: Christophe Fergeau 

On Fri, Mar 29, 2019 at 12:29:58PM +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/red-channel.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 121c7e475..4015c12df 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -70,6 +70,9 @@ struct RedChannelPrivate
>  uint32_t type;
>  uint32_t id;
>  
> +/* "core" interface to register events.
> + * Can be thread specific.
> + */
>  SpiceCoreInterfaceInternal *core;
>  gboolean handle_acks;
>  
> -- 
> 2.20.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common] build: Update verify.h to latest version

[Frediano Ziglio]

> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

> ---
>  common/verify.h | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/common/verify.h b/common/verify.h
> index 267de29..ecd8cdb 100644
> --- a/common/verify.h
> +++ b/common/verify.h
> @@ -1,6 +1,6 @@
>  /* Compile-time assert-like macros.
>  
> -   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
> +   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
>  
> This program is free software: you can redistribute it and/or modify
> it under the terms of the GNU Lesser General Public License as published
> by
> @@ -13,7 +13,7 @@
> GNU Lesser General Public License for more details.
>  
> You should have received a copy of the GNU Lesser General Public License
> -   along with this program.  If not, see .  */
> +   along with this program.  If not, see .
> */
>  
>  /* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
>  
> @@ -26,7 +26,7 @@
> here generates easier-to-read diagnostics when verify (R) fails.
>  
> Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
> -   This will likely be supported by future GCC versions, in C++ mode.
> +   This is supported by GCC 6.1.0 and later, in C++ mode.
>  
> Use this only with GCC.  If we were willing to slow 'configure'
> down we could also use it with other compilers, but since this
> @@ -36,9 +36,7 @@
>   && !defined __cplusplus)
>  # define _GL_HAVE__STATIC_ASSERT 1
>  #endif
> -/* The condition (99 < __GNUC__) is temporary, until we know about the
> -   first G++ release that supports static_assert.  */
> -#if (99 < __GNUC__) && defined __cplusplus
> +#if (6 <= __GNUC__) && defined __cplusplus
>  # define _GL_HAVE_STATIC_ASSERT 1
>  #endif
>  
> @@ -248,7 +246,12 @@ template 
>  /* Verify requirement R at compile-time, as a declaration without a
> trailing ';'.  */
>  
> -#define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> +#ifdef __GNUC__
> +# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> +#else
> +/* PGI barfs if R is long.  Play it safe.  */
> +# define verify(R) _GL_VERIFY (R, "verify (...)")
> +#endif
>  
>  #ifndef __has_builtin
>  # define __has_builtin(x) 0
> @@ -263,7 +266,7 @@ template 
>  # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
>  #elif 1200 <= _MSC_VER
>  # define assume(R) __assume (R)
> -#elif (defined lint \
> +#elif ((defined GCC_LINT || defined lint) \
> && (__has_builtin (__builtin_trap) \
> || 3 < __GNUC__ + (3 < __GNUC_MINOR__ + (4 <=
> || __GNUC_PATCHLEVEL__
>/* Doing it this way helps various packages when configured with
> @@ -271,7 +274,8 @@ template 
>   when 'assume' silences warnings even with older GCCs.  */
>  # define assume(R) ((R) ? (void) 0 : __builtin_trap ())
>  #else
> -# define assume(R) ((void) (0 && (R)))
> +  /* Some tools grok NOTREACHED, e.g., Oracle Studio 12.6.  */
> +# define assume(R) ((R) ? (void) 0 : /*NOTREACHED*/ (void) 0)
>  #endif
>  
>  /* @assert.h omit end@  */
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common] build: Update verify.h to latest version

[Christophe Fergeau]

Signed-off-by: Christophe Fergeau 
---
 common/verify.h | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/common/verify.h b/common/verify.h
index 267de29..ecd8cdb 100644
--- a/common/verify.h
+++ b/common/verify.h
@@ -1,6 +1,6 @@
 /* Compile-time assert-like macros.
 
-   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
+   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
 
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published by
@@ -13,7 +13,7 @@
GNU Lesser General Public License for more details.
 
You should have received a copy of the GNU Lesser General Public License
-   along with this program.  If not, see .  */
+   along with this program.  If not, see .  */
 
 /* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
 
@@ -26,7 +26,7 @@
here generates easier-to-read diagnostics when verify (R) fails.
 
Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
-   This will likely be supported by future GCC versions, in C++ mode.
+   This is supported by GCC 6.1.0 and later, in C++ mode.
 
Use this only with GCC.  If we were willing to slow 'configure'
down we could also use it with other compilers, but since this
@@ -36,9 +36,7 @@
  && !defined __cplusplus)
 # define _GL_HAVE__STATIC_ASSERT 1
 #endif
-/* The condition (99 < __GNUC__) is temporary, until we know about the
-   first G++ release that supports static_assert.  */
-#if (99 < __GNUC__) && defined __cplusplus
+#if (6 <= __GNUC__) && defined __cplusplus
 # define _GL_HAVE_STATIC_ASSERT 1
 #endif
 
@@ -248,7 +246,12 @@ template 
 /* Verify requirement R at compile-time, as a declaration without a
trailing ';'.  */
 
-#define verify(R) _GL_VERIFY (R, "verify (" #R ")")
+#ifdef __GNUC__
+# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
+#else
+/* PGI barfs if R is long.  Play it safe.  */
+# define verify(R) _GL_VERIFY (R, "verify (...)")
+#endif
 
 #ifndef __has_builtin
 # define __has_builtin(x) 0
@@ -263,7 +266,7 @@ template 
 # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
 #elif 1200 <= _MSC_VER
 # define assume(R) __assume (R)
-#elif (defined lint \
+#elif ((defined GCC_LINT || defined lint) \
&& (__has_builtin (__builtin_trap) \
|| 3 < __GNUC__ + (3 < __GNUC_MINOR__ + (4 <= 
__GNUC_PATCHLEVEL__
   /* Doing it this way helps various packages when configured with
@@ -271,7 +274,8 @@ template 
  when 'assume' silences warnings even with older GCCs.  */
 # define assume(R) ((R) ? (void) 0 : __builtin_trap ())
 #else
-# define assume(R) ((void) (0 && (R)))
+  /* Some tools grok NOTREACHED, e.g., Oracle Studio 12.6.  */
+# define assume(R) ((R) ? (void) 0 : /*NOTREACHED*/ (void) 0)
 #endif
 
 /* @assert.h omit end@  */
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 5/8] build: Update verify.h to latest version

[Christophe Fergeau]

On Fri, Mar 29, 2019 at 06:44:49AM -0400, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  common/verify.h | 24 ++--
> >  1 file changed, 14 insertions(+), 10 deletions(-)
> > 
> > diff --git a/common/verify.h b/common/verify.h
> > index 267de29..3f3dece 100644
> > --- a/common/verify.h
> > +++ b/common/verify.h
> > @@ -1,10 +1,10 @@
> >  /* Compile-time assert-like macros.
> >  
> > -   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
> > +   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
> >  
> > This program is free software: you can redistribute it and/or modify
> > it under the terms of the GNU Lesser General Public License as published
> > by
> > -   the Free Software Foundation; either version 2.1 of the License, or
> > +   the Free Software Foundation; either version 3 of the License, or
> > (at your option) any later version.
> >  
> > This program is distributed in the hope that it will be useful,
> 
> Still not compatible.

*sigh*, sorry about this, gnulib's verify module is lpglv2+
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=modules/verify;h=5216ce890dac58e596a27341e31258e5d6c0d702;hb=HEAD
Using gnulib-tool --import --lgpl=2 results in this s/2.1/3 change being gone
from the diff.

Christophe

> 
> > @@ -13,7 +13,7 @@
> > GNU Lesser General Public License for more details.
> >  
> > You should have received a copy of the GNU Lesser General Public License
> > -   along with this program.  If not, see .  
> > */
> > +   along with this program.  If not, see .
> > */
> >  
> >  /* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
> >  
> > @@ -26,7 +26,7 @@
> > here generates easier-to-read diagnostics when verify (R) fails.
> >  
> > Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
> > -   This will likely be supported by future GCC versions, in C++ mode.
> > +   This is supported by GCC 6.1.0 and later, in C++ mode.
> >  
> > Use this only with GCC.  If we were willing to slow 'configure'
> > down we could also use it with other compilers, but since this
> > @@ -36,9 +36,7 @@
> >   && !defined __cplusplus)
> >  # define _GL_HAVE__STATIC_ASSERT 1
> >  #endif
> > -/* The condition (99 < __GNUC__) is temporary, until we know about the
> > -   first G++ release that supports static_assert.  */
> > -#if (99 < __GNUC__) && defined __cplusplus
> > +#if (6 <= __GNUC__) && defined __cplusplus
> >  # define _GL_HAVE_STATIC_ASSERT 1
> >  #endif
> >  
> > @@ -248,7 +246,12 @@ template 
> >  /* Verify requirement R at compile-time, as a declaration without a
> > trailing ';'.  */
> >  
> > -#define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> > +#ifdef __GNUC__
> > +# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> > +#else
> > +/* PGI barfs if R is long.  Play it safe.  */
> > +# define verify(R) _GL_VERIFY (R, "verify (...)")
> > +#endif
> >  
> >  #ifndef __has_builtin
> >  # define __has_builtin(x) 0
> > @@ -263,7 +266,7 @@ template 
> >  # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
> >  #elif 1200 <= _MSC_VER
> >  # define assume(R) __assume (R)
> > -#elif (defined lint \
> > +#elif ((defined GCC_LINT || defined lint) \
> > && (__has_builtin (__builtin_trap) \
> > || 3 < __GNUC__ + (3 < __GNUC_MINOR__ + (4 <=
> > || __GNUC_PATCHLEVEL__
> >/* Doing it this way helps various packages when configured with
> > @@ -271,7 +274,8 @@ template 
> >   when 'assume' silences warnings even with older GCCs.  */
> >  # define assume(R) ((R) ? (void) 0 : __builtin_trap ())
> >  #else
> > -# define assume(R) ((void) (0 && (R)))
> > +  /* Some tools grok NOTREACHED, e.g., Oracle Studio 12.6.  */
> > +# define assume(R) ((R) ? (void) 0 : /*NOTREACHED*/ (void) 0)
> >  #endif
> >  
> >  /* @assert.h omit end@  */
> 
> Frediano


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Christophe Fergeau]

On Fri, Mar 29, 2019 at 06:51:54AM -0400, Frediano Ziglio wrote:
> > 
> > Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> > will never return.
> > 
> > Signed-off-by: Christophe Fergeau 
> 
> I prefer the "for" way. But by the way, this is not telling the compiler
> that spice_log (NOT g_log) is not returning but to call abort() after
> spice_log.

Since abort() does not return, overall result is that the compiler won't
think code after spice_critical() can be reached.

> I don't think the compiler will warn that with that flag the function
> it not returning, is not clear why you need these changes.
> Optimization? I don't think that calling an additional function
> and jumping back to original flow would change much.

I'm fairly sure I got warnings during some experiments which led me to
these changes, but I haven't been able to get them now.
One testcase which warns without these changes:

#include "log.h"
#include "stdbool.h"

int main(int argc, char **argv)
{
switch(argc) {
case 1:
spice_critical("foo");
default:
return 0;
}
}


$ LC_ALL=C gcc  -c$(pkg-config --cflags --libs glib-2.0 spice-protocol) -I 
common   -Wimplicit-fallthrough=5 ./fallthrough.c
In file included from ./fallthrough.c:1:
./fallthrough.c: In function 'main':
common/log.h:73:5: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
   73 | spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" 
format, ## __VA_ARGS__); \
  | 
^~
./fallthrough.c:8:25: note: in expansion of macro 'spice_critical'
8 | spice_critical("foo");
  | ^~
./fallthrough.c:9:17: note: here
9 | default:
  | ^~~

(I can add this to the log)

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Christophe Fergeau]

On Fri, Mar 29, 2019 at 07:18:35AM -0400, Frediano Ziglio wrote:
> > On Fri, Mar 29, 2019 at 06:57:59AM -0400, Frediano Ziglio wrote:
> > > > On Fri, Mar 29, 2019 at 11:30:46AM +0100, Christophe Fergeau wrote:
> > > > > Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> > > > > will never return.
> > > > > 
> > > > > Signed-off-by: Christophe Fergeau 
> > > > > ---
> > > > >  common/log.h | 5 +
> > > > >  1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/common/log.h b/common/log.h
> > > > > index 7c67e7a..1482358 100644
> > > > > --- a/common/log.h
> > > > > +++ b/common/log.h
> > > > > @@ -20,6 +20,7 @@
> > > > >  
> > > > >  #include 
> > > > >  #include 
> > > > > +#include 
> > > > >  #include 
> > > > >  #include 
> > > > >  
> > > > > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> > > > >  #define spice_return_if_fail(x) G_STMT_START {
> > > > >  \
> > > > >  if G_LIKELY(x) { } else {
> > > > >  \
> > > > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC,
> > > > >  "condition `%s' failed", #x); \
> > > > > +abort();
> > > > > \
> > > > >  return;
> > > > >  \
> > > > 
> > > > The 'return' statment is now unreachable code & can be removed -
> > > > surprised
> > > > the compiler didn't complain that its unreachable.
> > > > 
> > > 
> > > As OT I would also add that a "spice_return_if_fail" which don't return
> > > is confusing, basically it's a hidden assert.
> > 
> > Yeah it is a rather misleading name.  I see this code is importing glib.h
> > so I wonder why spice_return_if_fail needs to exist at all. Why not just
> > drop it and use g_return_if_fail from glib instead of reinventing the
> > wheel.  Or g_warn_if_fail if you want a log message, or g_assert if
> > you want a fatal error.
> > 
> > https://developer.gnome.org/glib/stable/glib-Warnings-and-Assertions.html#g-return-if-fail
> > 
> > Regards,
> > Daniel
> 
> Yes, this was discussed. The behaviour is different so it makes sense to keep
> them. For instance for use critical is fatal while in Glib not.
> Another difference is the informations they report.

Renaming spice_return_if_fail() to spice_assert_if_fail() or
something like this would make things much clearer. In the long run,
we should try to move away from these aborts when possible.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server] red-channel: Small comment on "core" field

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-channel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/server/red-channel.c b/server/red-channel.c
index 121c7e475..4015c12df 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -70,6 +70,9 @@ struct RedChannelPrivate
 uint32_t type;
 uint32_t id;
 
+/* "core" interface to register events.
+ * Can be thread specific.
+ */
 SpiceCoreInterfaceInternal *core;
 gboolean handle_acks;
 
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server] red-worker: Use bool for driver_cap_monitors_config

[Frediano Ziglio]

Easier to understand.

Signed-off-by: Frediano Ziglio 
---
 server/red-worker.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/red-worker.c b/server/red-worker.c
index bc63fc34f..a30148a55 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -83,7 +83,7 @@ struct RedWorker {
 RedStatCounter full_loop_counter;
 RedStatCounter total_loop_counter;
 
-int driver_cap_monitors_config;
+bool driver_cap_monitors_config;
 
 RedRecord *record;
 GMainLoop *loop;
@@ -646,7 +646,7 @@ static void handle_dev_monitors_config_async(void *opaque, 
void *payload)
 /* TODO: raise guest bug (requires added QXL interface) */
 goto async_complete;
 }
-worker->driver_cap_monitors_config = 1;
+worker->driver_cap_monitors_config = true;
 count = dev_monitors_config->count;
 max_allowed = dev_monitors_config->max_allowed;
 if (count == 0) {
@@ -752,7 +752,7 @@ static void handle_dev_driver_unload(void *opaque, void 
*payload)
 {
 RedWorker *worker = opaque;
 
-worker->driver_cap_monitors_config = 0;
+worker->driver_cap_monitors_config = false;
 }
 
 static
@@ -1076,7 +1076,7 @@ RedWorker* red_worker_new(QXLInstance *qxl)
 dispatcher_register_universal_handler(dispatcher, 
worker_dispatcher_record);
 }
 
-worker->driver_cap_monitors_config = 0;
+worker->driver_cap_monitors_config = false;
 char worker_str[SPICE_STAT_NODE_NAME_MAX];
 snprintf(worker_str, sizeof(worker_str), "display[%d]", worker->qxl->id & 
0xff);
 stat_init_node(>stat, reds, NULL, worker_str, TRUE);
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server] red-worker: Remove unused definitions

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-worker.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/server/red-worker.h b/server/red-worker.h
index 34c5b4aff..a107c178c 100644
--- a/server/red-worker.h
+++ b/server/red-worker.h
@@ -202,12 +202,6 @@ typedef struct RedWorkerMessageSetMouseMode {
 uint32_t mode;
 } RedWorkerMessageSetMouseMode;
 
-typedef struct RedWorkerMessageDisplayChannelCreate {
-} RedWorkerMessageDisplayChannelCreate;
-
-typedef struct RedWorkerMessageCursorChannelCreate {
-} RedWorkerMessageCursorChannelCreate;
-
 typedef struct RedWorkerMessageDestroySurfaceWait {
 uint32_t surface_id;
 } RedWorkerMessageDestroySurfaceWait;
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH linux/vd-agent 10/11] clipboard: only send release when no immediate grab

[Marc-André Lureau]

Hi

On Fri, Mar 29, 2019 at 10:17 AM Frediano Ziglio  wrote:
>
> >
> > Hi
> >
> > On Thu, Mar 28, 2019 at 5:30 PM Frediano Ziglio  wrote:
> > >
> > > >
> > > > From: Marc-André Lureau 
> > > >
> > > > Do not send a release event between two grabs, this helps with window
> > > > manager interaction issues on peer side.
> > > >
> > >
> > > I would explain which kind of issue this is supposed to fix.
> >
> > They react to clipboard events in different ways. The point is to
> > minimize the amount of events to avoid extra unnecessary interactions.
> >
> > In particular, on release event, some clipboard managers take the
> > grab. This creates a race with Spice which does a release between
> > grabs currently.
> >
>
> As clipboard managers are not using SPICE I suppose here you
> are talking about desktop clipboard release, not agent RELEASE
> message. But on the previous sentence we were speaking about
> agent RELEASE.

Yes, when a peer receives RELEASE, it will effectively release the
grab on its desktop, and this may make the desktop clipboard agent
react.

>
> > >
> > > > Advertise this behaviour via a capability introduced in spice-protocol
> > > > 0.12.16, so the client doesn't need to do some time-based filtering.
> > > >
> > > > (the capability shouldn't need to be negotiated, a client shouldn't
> > > > expect a release between two grabs)
> > > >
> > > > Signed-off-by: Marc-André Lureau 
> > >
> > > I suppose Jakub/Victor could do a much better review/test than me.
> > >
> > > > ---
> > > >  src/vdagent/clipboard.c | 12 ++--
> > > >  src/vdagent/x11.c   |  7 +++
> > > >  src/vdagentd/vdagentd.c |  1 +
> > > >  3 files changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c
> > > > index 9fb87e3..097c6ee 100644
> > > > --- a/src/vdagent/clipboard.c
> > > > +++ b/src/vdagent/clipboard.c
> > > > @@ -242,13 +242,13 @@ static void clipboard_owner_change_cb(GtkClipboard
> > > > *clipboard,
> > > >  return;
> > > >  }
> > > >
> > > > -if (sel->owner == OWNER_GUEST) {
> > > > -clipboard_new_owner(c, sel_id, OWNER_NONE);
> > > > -udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0,
> > > > NULL,
> > > > 0);
> > > > -}
> > > > -
> > > > -if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER)
> > > > +if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
> > > > +if (sel->owner == OWNER_GUEST) {
> > > > +clipboard_new_owner(c, sel_id, OWNER_NONE);
> > > > +udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0,
> > > > NULL, 0);
> > > > +}
> > > >  return;
> > > > +}
> > > >
> > > >  /* if there's a pending request for clipboard targets, cancel it */
> > > >  if (sel->last_targets_req)
> > > > diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> > > > index c2515a8..9409b53 100644
> > > > --- a/src/vdagent/x11.c
> > > > +++ b/src/vdagent/x11.c
> > > > @@ -530,11 +530,10 @@ static void vdagent_x11_handle_event(struct
> > > > vdagent_x11
> > > > *x11, XEvent event)
> > > >  if (ev.xfev.owner == x11->selection_window)
> > > >  return;
> > > >
> > > > -/* If the clipboard owner is changed we no longer own it */
> > >
> > > Why not keeping this comment?
> >
> > The comment is no longer valid with this change: if the clipboard
> > owner is changed, the agent may still own the grab (at the protocol
> > level).
> >
>
> Yes, this prove what I was saying in the previous e-mail, you are
> mixing desktop ownership and SPICE grab, if you consider from
> desktop prospective (which is using owner definition like in
> "ev.xfev.owner") the comment is still valid, but you are reading
> it with different definitions so it became invalid.
>
> > As you can see with the following line being removed:
> > >
> > > > -vdagent_x11_set_clipboard_owner(x11, selection, owner_none);
> > > > -
> > > > -if (ev.xfev.owner == None)
> > > > +if (ev.xfev.owner == None) {
> > > > +vdagent_x11_set_clipboard_owner(x11, selection, 
> > > > owner_none);
> > > >  return;
> > > > +}
> > > >
> > > >  /* Request the supported targets from the new owner */
> > > >  XConvertSelection(x11->display, ev.xfev.selection,
> > > >  x11->targets_atom,
> > > > diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> > > > index 72a3e13..683e5d3 100644
> > > > --- a/src/vdagentd/vdagentd.c
> > > > +++ b/src/vdagentd/vdagentd.c
> > > > @@ -133,6 +133,7 @@ static void send_capabilities(struct
> > > > vdagent_virtio_port
> > > > *vport,
> > > >  VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MAX_CLIPBOARD);
> > > >  VD_AGENT_SET_CAPABILITY(caps->caps, 
> > > > VD_AGENT_CAP_AUDIO_VOLUME_SYNC);
> > > >  VD_AGENT_SET_CAPABILITY(caps->caps,
> > > >  VD_AGENT_CAP_GRAPHICS_DEVICE_INFO);
> > > > +VD_AGENT_SET_CAPABILITY(caps->caps,
> > > > 

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Frediano Ziglio]

> On Fri, Mar 29, 2019 at 06:57:59AM -0400, Frediano Ziglio wrote:
> > > On Fri, Mar 29, 2019 at 11:30:46AM +0100, Christophe Fergeau wrote:
> > > > Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> > > > will never return.
> > > > 
> > > > Signed-off-by: Christophe Fergeau 
> > > > ---
> > > >  common/log.h | 5 +
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/common/log.h b/common/log.h
> > > > index 7c67e7a..1482358 100644
> > > > --- a/common/log.h
> > > > +++ b/common/log.h
> > > > @@ -20,6 +20,7 @@
> > > >  
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  
> > > > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> > > >  #define spice_return_if_fail(x) G_STMT_START {
> > > >  \
> > > >  if G_LIKELY(x) { } else {
> > > >  \
> > > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC,
> > > >  "condition `%s' failed", #x); \
> > > > +abort();
> > > > \
> > > >  return;
> > > >  \
> > > 
> > > The 'return' statment is now unreachable code & can be removed -
> > > surprised
> > > the compiler didn't complain that its unreachable.
> > > 
> > 
> > As OT I would also add that a "spice_return_if_fail" which don't return
> > is confusing, basically it's a hidden assert.
> 
> Yeah it is a rather misleading name.  I see this code is importing glib.h
> so I wonder why spice_return_if_fail needs to exist at all. Why not just
> drop it and use g_return_if_fail from glib instead of reinventing the
> wheel.  Or g_warn_if_fail if you want a log message, or g_assert if
> you want a fatal error.
> 
> https://developer.gnome.org/glib/stable/glib-Warnings-and-Assertions.html#g-return-if-fail
> 
> Regards,
> Daniel

Yes, this was discussed. The behaviour is different so it makes sense to keep
them. For instance for use critical is fatal while in Glib not.
Another difference is the informations they report.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL

[Marc-André Lureau]

Hi

On Fri, Mar 29, 2019 at 10:11 AM Frediano Ziglio  wrote:
>
> >
> > Hi
> >
> > On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio  wrote:
> > >
> > > >
> > > > ..Hi
> > > >
> > > > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio 
> > > > wrote:
> > > > > > The role of the grab message is to take ownership of the clipboard
> > > > > > (to
> > > > > > advertize clipboard data available). It may come at any time from
> > > > > > both
> > > > > > side, and override the current grab owner. It may come from the 
> > > > > > guest
> > > > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > > > Or it may come from the client (C-c in client side app), and the
> > > > > > agent
> > > > > > will grab the guest clipboard.
> > > > > >
> > > > >
> > > > > Yes, I realized this morning thinking about how clipboards works
> > > > > (very rusty in my mind).
> > > > > Instead of setting it you get the ownership that is you are willing
> > > > > to set a value on the clipboard but you defer setting it to avoid
> > > > > the expense of data copy.
> > > > > So, the old (original?) protocol was simply to sending entire data
> > > > > on every change, this avoided to loose the clipboard data entirely.
> > > >
> > > > I don't even remember that version. It looks like the original version
> > > > was already "on-demand"
> > > >
> > > >
> > > > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > > > Author: Arnon Gilboa 
> > > > Date:   Mon Oct 4 16:45:15 2010 +0200
> > > >
> > > > vd_agent: add protocol messages for clipboard/selection-owner model
> > > >
> > > > -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > > > application in our side ("we") got ownership of the clipboard.
> > > > -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > > > owns the clipboard (GRAB received), we notify the os we are the owner.
> > > > when we are asked by the os/app for the clipboard data, we send this
> > > > RE
> > > > QUEST msg to the other side.
> > > > -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > > > the clipboard, is now sent only in response to REQUEST.
> > > > -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > > > longer the owner of the clipboard (e.g. the owner app was closed).
> > > >
> > > > this patch will be followed by agent & client patches handling the
> > > > above messages.
> > > >
> > > >
> > > >
> > > > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to 
> > > > me.
> > > >
> > >
> > > I suppose the "now" in "the existing message for sending the clipboard, is
> > > now sent only in response to REQUEST" means that was changed.
> > >
> > > I personally think the code should be readable from a tarball/snapshot not
> > > pretending people to dig into 10 years of history. I'll try to find some
> > > time
> > > to put these lines into vd_agent.h.
> > >
> > > > > The current is: if we get new local clipboard data send the "grab"
> > > > > so remote knows that will have to read our data.
> > > >
> > > > yes
> > > >
> > > > > So either we have ownership/grab, meaning the data are remote
> > > > > waiting to get fetched or we don't have ownership and the remote
> > > > > should be grabbing as we have data to send (at least in a stable
> > > > > state).
> > > >
> > > >
> > > > That last sentence is confusing. There are 2 states the client can
> > > > "have the grab".
> > > >
> > > > 1. the client took the grab for the remote data: we are talking about
> > > > the client app in the client desktop environment
> > > > 2. the client took the grab, at the protocol level: it sent a grab
> > > > over the protocol to announce it can provide clipboard data to the
> > > > remote. In this case, the client app doesn't have the grab in the
> > > > client desktop environment, but another client application.
> > > >
> > > > In general, when talking about the protocol, "client has the grab"
> > > > means 2) to me, iow it can provide data to the remote.
> > > >
> > >
> > > I think I mean the opposite. The reason is that some application
> > > have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> > > the current local application (so spice-gtk/vd_agent) does NOT have the
> > > ownership and it's asking the remote to take to the application (so will
> > > remove the ownership from another remote application).
> > > From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> > > other side to grab (take ownership) of the clipboard so will be the
> > > remote to have the "grab", not the local.
> >
> >
> > I find it hard to understand you. The sequence of events is as such.
> >
> > A & B are client or remote exchangeably:
> > - an application on A takes the clipboard grab (== announce clipboard
> > data is available)
> > - A get notified by the desktop and sends a GRAB message to B side
> > - B receives GRAB, and takes the clipboard grab on B desktop side
> >
> > With this 

[Spice-devel] [PATCH spice-server v2 2/2] docs: Remove obsolete paragraph

[Frediano Ziglio]

ClientCbs were removed, all is automated in RedChannel.

Signed-off-by: Frediano Ziglio 
---
 docs/spice_threading_model.txt | 8 
 1 file changed, 8 deletions(-)

diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt
index 1ed82d4b4..62be39bbf 100644
--- a/docs/spice_threading_model.txt
+++ b/docs/spice_threading_model.txt
@@ -31,14 +31,6 @@ The main dispatcher is used to send requests to the main 
thread.
 The Qxl uses a dispatcher to send requests to the RedWorker which will forward
 to DisplayChannel/CursorChannel.
 
-RedClient may call some RedChannelClient functions using some callbacks
-registered inside ClientCbs. Usually these callbacks are functions that do the
-job directly if the RedChannel is running in the main thread or they use a
-dispatcher to do the job in the right thread. Currently there are 3 callbacks:
-connect, disconnect and migrate. Connect and migrate are asynchronous (the job
-is done while the current thread is doing something else) while disconnect is
-synchronous (the main thread will wait for termination).
-
 One aspect to take into consideration is the event scheduling. SPICE uses some
 `SpiceCoreInterface` to handle events. As the events will be handled from a
 thread based on the core interface you have to use the correct core. Each
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v2 1/2] docs: Add some notes on event scheduling and threading

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 docs/spice_threading_model.txt | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt
index 9351141c8..1ed82d4b4 100644
--- a/docs/spice_threading_model.txt
+++ b/docs/spice_threading_model.txt
@@ -39,6 +39,20 @@ connect, disconnect and migrate. Connect and migrate are 
asynchronous (the job
 is done while the current thread is doing something else) while disconnect is
 synchronous (the main thread will wait for termination).
 
+One aspect to take into consideration is the event scheduling. SPICE uses some
+`SpiceCoreInterface` to handle events. As the events will be handled from a
+thread based on the core interface you have to use the correct core. Each
+channel has an associated core interface which can be retrieved using
+`red_channel_get_core_interface`. There's also a main core interface you can 
get
+using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*`
+functions use the main core interface.
+Even though multiple channel types run in the main thread and so could use
+directly the main code interface, for coherence, rule simplicity and to allows
+the code to be moved in a separate thread easily if needed, it's advisable to
+use the channel core interface (that will be the main core interface in this
+case).
+Currently character devices and not channel code runs in the main thread.
+
 Reference counting and ownership
 
 ->  pointer
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-streaming-agent] Add support log logging statistics from plugins

[Frediano Ziglio]

ping

> 
> Allows the plugins to add information to the log.
> 
> Signed-off-by: Frediano Ziglio 
> ---
> Not entirely happy to export a kind of C function, any suggestion
> welcome
> ---
>  include/spice-streaming-agent/plugin.hpp |  5 +
>  src/concrete-agent.cpp   | 16 ++--
>  src/concrete-agent.hpp   |  6 ++
>  src/frame-log.cpp|  9 +
>  src/frame-log.hpp|  2 ++
>  src/spice-streaming-agent.cpp|  8 ++--
>  6 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/include/spice-streaming-agent/plugin.hpp
> b/include/spice-streaming-agent/plugin.hpp
> index 3b265d9..a51f060 100644
> --- a/include/spice-streaming-agent/plugin.hpp
> +++ b/include/spice-streaming-agent/plugin.hpp
> @@ -116,6 +116,11 @@ public:
>   * \todo passing options to entry point instead?
>   */
>  virtual const ConfigureOption* Options() const = 0;
> +/*!
> + * Write something in the log.
> + */
> +__attribute__ ((format (printf, 2, 3)))
> +virtual void LogStat(const char* format, ...) = 0;
>  };
>  
>  typedef bool PluginInitFunc(spice::streaming_agent::Agent* agent);
> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
> index f94aead..d279656 100644
> --- a/src/concrete-agent.cpp
> +++ b/src/concrete-agent.cpp
> @@ -5,13 +5,15 @@
>   */
>  
>  #include 
> +#include "concrete-agent.hpp"
> +#include "frame-log.hpp"
> +
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> -
> -#include "concrete-agent.hpp"
> +#include 
>  
>  using namespace spice::streaming_agent;
>  
> @@ -145,3 +147,13 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture(const
> std::set  }
>  return nullptr;
>  }
> +
> +void ConcreteAgent::LogStat(const char* format, ...)
> +{
> +if (logger) {
> +va_list ap;
> +va_start(ap, format);
> +logger->log_statv(format, ap);
> +va_end(ap);
> +}
> +}
> diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
> index 99dcf54..aa4d6aa 100644
> --- a/src/concrete-agent.hpp
> +++ b/src/concrete-agent.hpp
> @@ -14,6 +14,8 @@
>  namespace spice {
>  namespace streaming_agent {
>  
> +class FrameLog;
> +
>  struct ConcreteConfigureOption: ConfigureOption
>  {
>  ConcreteConfigureOption(const char *name, const char *value)
> @@ -33,11 +35,15 @@ public:
>  // pointer must remain valid
>  void AddOption(const char *name, const char *value);
>  FrameCapture *GetBestFrameCapture(const std::set&
>  codecs);
> +void SetFrameLog(FrameLog *logger) { this->logger = logger; }
> +__attribute__ ((format (printf, 2, 3)))
> +void LogStat(const char* format, ...) override;
>  private:
>  bool PluginVersionIsCompatible(unsigned pluginVersion) const;
>  void LoadPlugin(const std::string _filename);
>  std::vector> plugins;
>  std::vector options;
> +FrameLog *logger = nullptr;
>  };
>  
>  }} // namespace spice::streaming_agent
> diff --git a/src/frame-log.cpp b/src/frame-log.cpp
> index 62fffc3..db6b652 100644
> --- a/src/frame-log.cpp
> +++ b/src/frame-log.cpp
> @@ -52,6 +52,15 @@ void FrameLog::log_stat(const char* format, ...)
>  }
>  }
>  
> +void FrameLog::log_statv(const char* format, va_list ap)
> +{
> +if (log_file) {
> +fprintf(log_file, "%" PRIu64 ": ", get_time());
> +vfprintf(log_file, format, ap);
> +fputc('\n', log_file);
> +}
> +}
> +
>  void FrameLog::log_frame(const void* buffer, size_t buffer_size)
>  {
>  if (log_file) {
> diff --git a/src/frame-log.hpp b/src/frame-log.hpp
> index 8503345..a104723 100644
> --- a/src/frame-log.hpp
> +++ b/src/frame-log.hpp
> @@ -24,6 +24,8 @@ public:
>  
>  __attribute__ ((format (printf, 2, 3)))
>  void log_stat(const char* format, ...);
> +__attribute__ ((format (printf, 2, 0)))
> +void log_statv(const char* format, va_list ap);
>  void log_frame(const void* buffer, size_t buffer_size);
>  
>  static uint64_t get_time();
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 9507a54..f262118 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -401,13 +401,15 @@ int main(int argc, char* argv[])
>  register_interrupts();
>  
>  try {
> +FrameLog frame_log(log_filename, log_binary, log_frames);
> +
> +agent.SetFrameLog(_log);
> +
>  // register built-in plugins
>  MjpegPlugin::Register();
>  
>  agent.LoadPlugins(pluginsdir);
>  
> -FrameLog frame_log(log_filename, log_binary, log_frames);
> -
>  for (const std::string& arg: old_args) {
>  frame_log.log_stat("Args: %s", arg.c_str());
>  }
> @@ -419,8 +421,10 @@ int main(int argc, char* argv[])
>  cursor_updater.detach();
>  
>  do_capture(stream_port, frame_log);
> +agent.SetFrameLog(nullptr);
>  }
>  catch 

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Daniel P . Berrangé]

On Fri, Mar 29, 2019 at 06:57:59AM -0400, Frediano Ziglio wrote:
> > On Fri, Mar 29, 2019 at 11:30:46AM +0100, Christophe Fergeau wrote:
> > > Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> > > will never return.
> > > 
> > > Signed-off-by: Christophe Fergeau 
> > > ---
> > >  common/log.h | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/common/log.h b/common/log.h
> > > index 7c67e7a..1482358 100644
> > > --- a/common/log.h
> > > +++ b/common/log.h
> > > @@ -20,6 +20,7 @@
> > >  
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  
> > > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> > >  #define spice_return_if_fail(x) G_STMT_START {  \
> > >  if G_LIKELY(x) { } else {   \
> > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC,
> > >  "condition `%s' failed", #x); \
> > > +abort();
> > > \
> > >  return; \
> > 
> > The 'return' statment is now unreachable code & can be removed - surprised
> > the compiler didn't complain that its unreachable.
> > 
> 
> As OT I would also add that a "spice_return_if_fail" which don't return
> is confusing, basically it's a hidden assert.

Yeah it is a rather misleading name.  I see this code is importing glib.h
so I wonder why spice_return_if_fail needs to exist at all. Why not just
drop it and use g_return_if_fail from glib instead of reinventing the
wheel.  Or g_warn_if_fail if you want a log message, or g_assert if
you want a fatal error.

https://developer.gnome.org/glib/stable/glib-Warnings-and-Assertions.html#g-return-if-fail

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Frediano Ziglio]

> On Fri, Mar 29, 2019 at 11:30:46AM +0100, Christophe Fergeau wrote:
> > Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> > will never return.
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  common/log.h | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/common/log.h b/common/log.h
> > index 7c67e7a..1482358 100644
> > --- a/common/log.h
> > +++ b/common/log.h
> > @@ -20,6 +20,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> >  #define spice_return_if_fail(x) G_STMT_START {  \
> >  if G_LIKELY(x) { } else {   \
> >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC,
> >  "condition `%s' failed", #x); \
> > +abort();
> > \
> >  return; \
> 
> The 'return' statment is now unreachable code & can be removed - surprised
> the compiler didn't complain that its unreachable.
> 

As OT I would also add that a "spice_return_if_fail" which don't return
is confusing, basically it's a hidden assert.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Daniel P . Berrangé]

On Fri, Mar 29, 2019 at 11:30:46AM +0100, Christophe Fergeau wrote:
> Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> will never return.
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  common/log.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/common/log.h b/common/log.h
> index 7c67e7a..1482358 100644
> --- a/common/log.h
> +++ b/common/log.h
> @@ -20,6 +20,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
>  #define spice_return_if_fail(x) G_STMT_START {  \
>  if G_LIKELY(x) { } else {   \
>  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition 
> `%s' failed", #x); \
> +abort(); 
>   \
>  return; \

The 'return' statment is now unreachable code & can be removed - surprised
the compiler didn't complain that its unreachable.

>  }   \
>  } G_STMT_END
> @@ -49,6 +51,7 @@ void spice_log(GLogLevelFlags log_level,
>  #define spice_return_val_if_fail(x, val) G_STMT_START { \
>  if G_LIKELY(x) { } else {   \
>  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, 
> "condition `%s' failed", #x); \
> +abort(); 
>  \
>  return (val);   \

Again here.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Frediano Ziglio]

> 
> Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
> will never return.
> 
> Signed-off-by: Christophe Fergeau 

I prefer the "for" way. But by the way, this is not telling the compiler
that spice_log (NOT g_log) is not returning but to call abort() after
spice_log.
I don't think the compiler will warn that with that flag the function
it not returning, is not clear why you need these changes.
Optimization? I don't think that calling an additional function
and jumping back to original flow would change much.

> ---
>  common/log.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/common/log.h b/common/log.h
> index 7c67e7a..1482358 100644
> --- a/common/log.h
> +++ b/common/log.h
> @@ -20,6 +20,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
>  #define spice_return_if_fail(x) G_STMT_START {  \
>  if G_LIKELY(x) { } else {   \
>  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition
>  `%s' failed", #x); \
> +abort();
> \
>  return; \
>  }   \
>  } G_STMT_END
> @@ -49,6 +51,7 @@ void spice_log(GLogLevelFlags log_level,
>  #define spice_return_val_if_fail(x, val) G_STMT_START { \
>  if G_LIKELY(x) { } else {   \
>  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__,
>  "condition `%s' failed", #x); \
> +abort();
> \
>  return (val);   \
>  }   \
>  } G_STMT_END
> @@ -71,10 +74,12 @@ void spice_log(GLogLevelFlags log_level,
>  
>  #define spice_critical(format, ...) G_STMT_START {
>  \
>  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format,
>  ## __VA_ARGS__); \
> +abort();
> \
>  } G_STMT_END
>  
>  #define spice_error(format, ...) G_STMT_START { \
>  spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, "" format, ##
>  __VA_ARGS__); \
> +abort();
> \
>  } G_STMT_END
>  
>  #define spice_warn_if_fail(x) G_STMT_START {\

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 6/8] test-marshallers: Fix header guard

[Frediano Ziglio]

> 
> test-marshallers.h is missing a #define _H_TEST_MARSHALLERS in order to
> prevent multiple #include for the same header.
> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

> ---
>  tests/test-marshallers.h | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
> index 4eab90f..7686067 100644
> --- a/tests/test-marshallers.h
> +++ b/tests/test-marshallers.h
> @@ -1,7 +1,8 @@
> +#ifndef H_SPICE_COMMON_TEST_MARSHALLERS
> +#define H_SPICE_COMMON_TEST_MARSHALLERS
> +
>  #include 
>  
> -#ifndef _H_TEST_MARSHALLERS
> -
>  typedef struct {
>  uint32_t data_size;
>  uint8_t dummy_byte;
> @@ -26,5 +27,5 @@ typedef struct {
>  uint8_t data[0];
>  } SpiceMsgMainLenMessage;
>  
> -#endif /* _H_TEST_MARSHALLERS */
> +#endif /* H_SPICE_COMMON_TEST_MARSHALLERS */
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 4/8] build: Add missing G_GNUC_PRINTF annotations

[Frediano Ziglio]

> 
> They were suggested by gcc when using -Wsuggest-attribute=format
> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

> ---
>  common/log.c | 1 +
>  tests/test-logging.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/common/log.c b/common/log.c
> index b73da71..ce162a1 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -33,6 +33,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
>  recorder_dump_on_common_signals(0, 0);
>  }
>  
> +G_GNUC_PRINTF(5, 0)
>  static void spice_logv(const char *log_domain,
> GLogLevelFlags log_level,
> const char *strloc,
> diff --git a/tests/test-logging.c b/tests/test-logging.c
> index 6a79ca9..32b0c33 100644
> --- a/tests/test-logging.c
> +++ b/tests/test-logging.c
> @@ -27,6 +27,7 @@
>  
>  #define OTHER_LOG_DOMAIN "Other"
>  #define LOG_OTHER_HELPER(suffix, level)
>  \
> +G_GNUC_PRINTF(1, 2)
> \
>  static void G_PASTE(other_, suffix)(const gchar *format, ...)
>  \
>  {
>  \
>  va_list args;
>  \
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common v2 5/8] build: Update verify.h to latest version

[Frediano Ziglio]

> 
> Signed-off-by: Christophe Fergeau 
> ---
>  common/verify.h | 24 ++--
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/common/verify.h b/common/verify.h
> index 267de29..3f3dece 100644
> --- a/common/verify.h
> +++ b/common/verify.h
> @@ -1,10 +1,10 @@
>  /* Compile-time assert-like macros.
>  
> -   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
> +   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
>  
> This program is free software: you can redistribute it and/or modify
> it under the terms of the GNU Lesser General Public License as published
> by
> -   the Free Software Foundation; either version 2.1 of the License, or
> +   the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
>  
> This program is distributed in the hope that it will be useful,

Still not compatible.

> @@ -13,7 +13,7 @@
> GNU Lesser General Public License for more details.
>  
> You should have received a copy of the GNU Lesser General Public License
> -   along with this program.  If not, see .  */
> +   along with this program.  If not, see .
> */
>  
>  /* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
>  
> @@ -26,7 +26,7 @@
> here generates easier-to-read diagnostics when verify (R) fails.
>  
> Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
> -   This will likely be supported by future GCC versions, in C++ mode.
> +   This is supported by GCC 6.1.0 and later, in C++ mode.
>  
> Use this only with GCC.  If we were willing to slow 'configure'
> down we could also use it with other compilers, but since this
> @@ -36,9 +36,7 @@
>   && !defined __cplusplus)
>  # define _GL_HAVE__STATIC_ASSERT 1
>  #endif
> -/* The condition (99 < __GNUC__) is temporary, until we know about the
> -   first G++ release that supports static_assert.  */
> -#if (99 < __GNUC__) && defined __cplusplus
> +#if (6 <= __GNUC__) && defined __cplusplus
>  # define _GL_HAVE_STATIC_ASSERT 1
>  #endif
>  
> @@ -248,7 +246,12 @@ template 
>  /* Verify requirement R at compile-time, as a declaration without a
> trailing ';'.  */
>  
> -#define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> +#ifdef __GNUC__
> +# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
> +#else
> +/* PGI barfs if R is long.  Play it safe.  */
> +# define verify(R) _GL_VERIFY (R, "verify (...)")
> +#endif
>  
>  #ifndef __has_builtin
>  # define __has_builtin(x) 0
> @@ -263,7 +266,7 @@ template 
>  # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
>  #elif 1200 <= _MSC_VER
>  # define assume(R) __assume (R)
> -#elif (defined lint \
> +#elif ((defined GCC_LINT || defined lint) \
> && (__has_builtin (__builtin_trap) \
> || 3 < __GNUC__ + (3 < __GNUC_MINOR__ + (4 <=
> || __GNUC_PATCHLEVEL__
>/* Doing it this way helps various packages when configured with
> @@ -271,7 +274,8 @@ template 
>   when 'assume' silences warnings even with older GCCs.  */
>  # define assume(R) ((R) ? (void) 0 : __builtin_trap ())
>  #else
> -# define assume(R) ((void) (0 && (R)))
> +  /* Some tools grok NOTREACHED, e.g., Oracle Studio 12.6.  */
> +# define assume(R) ((R) ? (void) 0 : /*NOTREACHED*/ (void) 0)
>  #endif
>  
>  /* @assert.h omit end@  */

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 4/8] build: Add missing G_GNUC_PRINTF annotations

[Christophe Fergeau]

They were suggested by gcc when using -Wsuggest-attribute=format

Signed-off-by: Christophe Fergeau 
---
 common/log.c | 1 +
 tests/test-logging.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/common/log.c b/common/log.c
index b73da71..ce162a1 100644
--- a/common/log.c
+++ b/common/log.c
@@ -33,6 +33,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
 recorder_dump_on_common_signals(0, 0);
 }
 
+G_GNUC_PRINTF(5, 0)
 static void spice_logv(const char *log_domain,
GLogLevelFlags log_level,
const char *strloc,
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 6a79ca9..32b0c33 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -27,6 +27,7 @@
 
 #define OTHER_LOG_DOMAIN "Other"
 #define LOG_OTHER_HELPER(suffix, level)
  \
+G_GNUC_PRINTF(1, 2)
  \
 static void G_PASTE(other_, suffix)(const gchar *format, ...)  
  \
 {  
  \
 va_list args;  
  \
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 8/8] log: Let gcc know about the logging macros which abort

[Christophe Fergeau]

Without the added abort(), it cannot know g_log(G_LOG_LEVEL_CRITICAL)
will never return.

Signed-off-by: Christophe Fergeau 
---
 common/log.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/common/log.h b/common/log.h
index 7c67e7a..1482358 100644
--- a/common/log.h
+++ b/common/log.h
@@ -20,6 +20,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
 #define spice_return_if_fail(x) G_STMT_START {  \
 if G_LIKELY(x) { } else {   \
 spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition 
`%s' failed", #x); \
+abort();   
\
 return; \
 }   \
 } G_STMT_END
@@ -49,6 +51,7 @@ void spice_log(GLogLevelFlags log_level,
 #define spice_return_val_if_fail(x, val) G_STMT_START { \
 if G_LIKELY(x) { } else {   \
 spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "condition 
`%s' failed", #x); \
+abort();   
   \
 return (val);   \
 }   \
 } G_STMT_END
@@ -71,10 +74,12 @@ void spice_log(GLogLevelFlags log_level,
 
 #define spice_critical(format, ...) G_STMT_START {  \
 spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format, ## 
__VA_ARGS__); \
+abort();   
 \
 } G_STMT_END
 
 #define spice_error(format, ...) G_STMT_START { \
 spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, "" format, ## 
__VA_ARGS__); \
+abort();   
  \
 } G_STMT_END
 
 #define spice_warn_if_fail(x) G_STMT_START {\
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 6/8] test-marshallers: Fix header guard

[Christophe Fergeau]

test-marshallers.h is missing a #define _H_TEST_MARSHALLERS in order to
prevent multiple #include for the same header.

Signed-off-by: Christophe Fergeau 
---
 tests/test-marshallers.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
index 4eab90f..7686067 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -1,7 +1,8 @@
+#ifndef H_SPICE_COMMON_TEST_MARSHALLERS
+#define H_SPICE_COMMON_TEST_MARSHALLERS
+
 #include 
 
-#ifndef _H_TEST_MARSHALLERS
-
 typedef struct {
 uint32_t data_size;
 uint8_t dummy_byte;
@@ -26,5 +27,5 @@ typedef struct {
 uint8_t data[0];
 } SpiceMsgMainLenMessage;
 
-#endif /* _H_TEST_MARSHALLERS */
+#endif /* H_SPICE_COMMON_TEST_MARSHALLERS */
 
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 3/8] lz: Don't try to print uninitialized variable

[Christophe Fergeau]

encoder->type is only going to be set by lz_set_sizes() after the
error() call. We can use 'type' directly which is what encoder->type is
going to be set to.

Signed-off-by: Christophe Fergeau 
Acked-by: Frediano Ziglio 
---
 common/lz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lz.c b/common/lz.c
index 167e118..f92c638 100644
--- a/common/lz.c
+++ b/common/lz.c
@@ -616,7 +616,7 @@ void lz_decode_begin(LzContext *lz, uint8_t *io_ptr, 
unsigned int num_io_bytes,
 
 int type = decode_32(encoder);
 if (type <= LZ_IMAGE_TYPE_INVALID || type > LZ_IMAGE_TYPE_A8) {
-encoder->usr->error(encoder->usr, "invalid lz type %d\n", 
encoder->type);
+encoder->usr->error(encoder->usr, "invalid lz type %d\n", type);
 }
 int width = decode_32(encoder);
 int height = decode_32(encoder);
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 2/8] backtrace: Add missing include

[Christophe Fergeau]

This fixes a warning about missing prototype for backtrace()

Signed-off-by: Christophe Fergeau 
Acked-by: Frediano Ziglio 
---
 common/backtrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/backtrace.c b/common/backtrace.c
index c4edde1..ff72d1b 100644
--- a/common/backtrace.c
+++ b/common/backtrace.c
@@ -25,6 +25,8 @@
 #include 
 #endif
 
+#include "backtrace.h"
+
 #include 
 #include 
 #include 
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 7/8] quic: Fix QUIC_VERSION definition

[Christophe Fergeau]

QUIC_VERSION_MINOR is never used.. Set QUIC_VERSION_MINOR to the same
version as QUIC_VERSION_MAJOR to avoid breaking backwards compatibility,
and fix the QUIC_VERSION macro.

Signed-off-by: Christophe Fergeau 
Acked-by: Frediano Ziglio 
---
 common/quic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/quic.c b/common/quic.c
index 1760274..f91b23f 100644
--- a/common/quic.c
+++ b/common/quic.c
@@ -31,8 +31,8 @@
 /* ASCII "QUIC" */
 #define QUIC_MAGIC 0x43495551
 #define QUIC_VERSION_MAJOR 0U
-#define QUIC_VERSION_MINOR 1U
-#define QUIC_VERSION ((QUIC_VERSION_MAJOR << 16) | (QUIC_VERSION_MAJOR & 
0x))
+#define QUIC_VERSION_MINOR 0U
+#define QUIC_VERSION ((QUIC_VERSION_MAJOR << 16) | (QUIC_VERSION_MINOR & 
0x))
 
 typedef uint8_t BYTE;
 
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 1/8] canvas_base: Fix variable shadowing warning

[Christophe Fergeau]

canvas_base.c is #included by spice-common users. They currently don't
enable this warning, but if/when they do, we don't want code from
spice-common to trigger it.

Signed-off-by: Christophe Fergeau 
Acked-by: Frediano Ziglio 
---
 common/canvas_base.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/canvas_base.c b/common/canvas_base.c
index 9ffca3e..00a8801 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -1880,11 +1880,11 @@ static void canvas_clip_pixman(CanvasBase *canvas,
 uint32_t n = clip->rects->num_rects;
 SpiceRect *now = clip->rects->rects;
 
-pixman_region32_t clip;
+pixman_region32_t pixman_clip;
 
-if (spice_pixman_region32_init_rects(, now, n)) {
-pixman_region32_intersect(dest_region, dest_region, );
-pixman_region32_fini();
+if (spice_pixman_region32_init_rects(_clip, now, n)) {
+pixman_region32_intersect(dest_region, dest_region, _clip);
+pixman_region32_fini(_clip);
 }
 
 break;
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-common v2 5/8] build: Update verify.h to latest version

[Christophe Fergeau]

Signed-off-by: Christophe Fergeau 
---
 common/verify.h | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/common/verify.h b/common/verify.h
index 267de29..3f3dece 100644
--- a/common/verify.h
+++ b/common/verify.h
@@ -1,10 +1,10 @@
 /* Compile-time assert-like macros.
 
-   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
+   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
 
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published by
-   the Free Software Foundation; either version 2.1 of the License, or
+   the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
 
This program is distributed in the hope that it will be useful,
@@ -13,7 +13,7 @@
GNU Lesser General Public License for more details.
 
You should have received a copy of the GNU Lesser General Public License
-   along with this program.  If not, see .  */
+   along with this program.  If not, see .  */
 
 /* Written by Paul Eggert, Bruno Haible, and Jim Meyering.  */
 
@@ -26,7 +26,7 @@
here generates easier-to-read diagnostics when verify (R) fails.
 
Define _GL_HAVE_STATIC_ASSERT to 1 if static_assert works as per C++11.
-   This will likely be supported by future GCC versions, in C++ mode.
+   This is supported by GCC 6.1.0 and later, in C++ mode.
 
Use this only with GCC.  If we were willing to slow 'configure'
down we could also use it with other compilers, but since this
@@ -36,9 +36,7 @@
  && !defined __cplusplus)
 # define _GL_HAVE__STATIC_ASSERT 1
 #endif
-/* The condition (99 < __GNUC__) is temporary, until we know about the
-   first G++ release that supports static_assert.  */
-#if (99 < __GNUC__) && defined __cplusplus
+#if (6 <= __GNUC__) && defined __cplusplus
 # define _GL_HAVE_STATIC_ASSERT 1
 #endif
 
@@ -248,7 +246,12 @@ template 
 /* Verify requirement R at compile-time, as a declaration without a
trailing ';'.  */
 
-#define verify(R) _GL_VERIFY (R, "verify (" #R ")")
+#ifdef __GNUC__
+# define verify(R) _GL_VERIFY (R, "verify (" #R ")")
+#else
+/* PGI barfs if R is long.  Play it safe.  */
+# define verify(R) _GL_VERIFY (R, "verify (...)")
+#endif
 
 #ifndef __has_builtin
 # define __has_builtin(x) 0
@@ -263,7 +266,7 @@ template 
 # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
 #elif 1200 <= _MSC_VER
 # define assume(R) __assume (R)
-#elif (defined lint \
+#elif ((defined GCC_LINT || defined lint) \
&& (__has_builtin (__builtin_trap) \
|| 3 < __GNUC__ + (3 < __GNUC_MINOR__ + (4 <= 
__GNUC_PATCHLEVEL__
   /* Doing it this way helps various packages when configured with
@@ -271,7 +274,8 @@ template 
  when 'assume' silences warnings even with older GCCs.  */
 # define assume(R) ((R) ? (void) 0 : __builtin_trap ())
 #else
-# define assume(R) ((void) (0 && (R)))
+  /* Some tools grok NOTREACHED, e.g., Oracle Studio 12.6.  */
+# define assume(R) ((R) ? (void) 0 : /*NOTREACHED*/ (void) 0)
 #endif
 
 /* @assert.h omit end@  */
-- 
2.21.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common 6/8] test-marshallers: Fix header guard

[Christophe Fergeau]

On Thu, Mar 28, 2019 at 01:54:01PM -0400, Frediano Ziglio wrote:
> > 
> > test-marshallers.h is missing a #define _H_TEST_MARSHALLERS in order to
> > prevent multiple #include for the same header.
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  tests/test-marshallers.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
> > index 4eab90f..5b113b7 100644
> > --- a/tests/test-marshallers.h
> > +++ b/tests/test-marshallers.h
> > @@ -1,6 +1,7 @@
> >  #include 
> >  
> 
> OT: why this is outside the guard?
> 
> >  #ifndef _H_TEST_MARSHALLERS
> > +#define _H_TEST_MARSHALLERS
> >  
> 
> OT 2: Why not H_SPICE_COMMON_ prefix?

I'll update the patch to fix these as well.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common 1/8] canvas_base: Fix variable shadowing warning

[Frediano Ziglio]

> canvas_base.c is #included by spice-common users. They currently don't
> enable this warning, but if/when they do, we don't want code from
> spice-common to trigger it.
> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

> ---
>  common/canvas_base.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/common/canvas_base.c b/common/canvas_base.c
> index 9ffca3e..00a8801 100644
> --- a/common/canvas_base.c
> +++ b/common/canvas_base.c
> @@ -1880,11 +1880,11 @@ static void canvas_clip_pixman(CanvasBase *canvas,
>  uint32_t n = clip->rects->num_rects;
>  SpiceRect *now = clip->rects->rects;
>  
> -pixman_region32_t clip;
> +pixman_region32_t pixman_clip;
>  
> -if (spice_pixman_region32_init_rects(, now, n)) {
> -pixman_region32_intersect(dest_region, dest_region, );
> -pixman_region32_fini();
> +if (spice_pixman_region32_init_rects(_clip, now, n)) {
> +pixman_region32_intersect(dest_region, dest_region,
> _clip);
> +pixman_region32_fini(_clip);
>  }
>  
>  break;

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common 8/8] log: Let gcc know about the logging macros which abort

[Frediano Ziglio]

> On Thu, Mar 28, 2019 at 02:02:58PM -0400, Frediano Ziglio wrote:
> > > 
> > > The for(;;) hack was taken from glib's logging macros.
> > > 
> > > Signed-off-by: Christophe Fergeau 
> > > ---
> > >  common/log.h | 8 
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/common/log.h b/common/log.h
> > > index 7c67e7a..b397306 100644
> > > --- a/common/log.h
> > > +++ b/common/log.h
> > > @@ -20,6 +20,7 @@
> > >  
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  
> > > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> > >  #define spice_return_if_fail(x) G_STMT_START {
> > >  \
> > >  if G_LIKELY(x) { } else {
> > >  \
> > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC,
> > >  "condition
> > >  `%s' failed", #x); \
> > > +abort();
> > > \
> > >  return;
> > >  \
> > >  }
> > >  \
> > >  } G_STMT_END
> > > @@ -49,6 +51,7 @@ void spice_log(GLogLevelFlags log_level,
> > >  #define spice_return_val_if_fail(x, val) G_STMT_START {
> > >  \
> > >  if G_LIKELY(x) { } else {
> > >  \
> > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__,
> > >  "condition `%s' failed", #x); \
> > > +abort();
> > > \
> > >  return (val);
> > >  \
> > >  }
> > >  \
> > >  } G_STMT_END
> > > @@ -69,12 +72,17 @@ void spice_log(GLogLevelFlags log_level,
> > >  spice_log(G_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, ""
> > >  format, ##
> > >  __VA_ARGS__); \
> > >  } G_STMT_END
> > >  
> > > +/* for(;;) ; so that GCC knows that control doesn't go past g_error().
> > 
> > g_error? copy error?
> > 
> > > + * Put space before ending semicolon to avoid C++ build warnings.
> > > + */
> > >  #define spice_critical(format, ...) G_STMT_START {
> > >  \
> > >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, ""
> > >  format,
> > >  ## __VA_ARGS__); \
> > > +for (;;) ;
> > 
> > I suppose you can use "for(;;) continue;" and remove the comment (that
> > "continue" was an old suggestion I had, I agree with C++ warning like
> > I agreed at that time with the continue).
> > 
> > Why some are for and some abort?
> 
> Actually, I can't remember, and I haven't been able to reproduce the
> warnings I wanted to fix. I changed all to abort(), which should be
> enough.
> 
> Christophe
> 

I would personally change all to for to avoid the additional include.
If you don't are able to reproduce why "fixing" ?

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common 5/8] build: Update verify.h to latest version

[Christophe Fergeau]

On Thu, Mar 28, 2019 at 01:56:06PM -0400, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  common/verify.h | 24 +++-
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/common/verify.h b/common/verify.h
> > index 267de29..b2e5f64 100644
> > --- a/common/verify.h
> > +++ b/common/verify.h
> > @@ -1,19 +1,19 @@
> >  /* Compile-time assert-like macros.
> >  
> > -   Copyright (C) 2005-2006, 2009-2016 Free Software Foundation, Inc.
> > +   Copyright (C) 2005-2006, 2009-2019 Free Software Foundation, Inc.
> >  
> > This program is free software: you can redistribute it and/or modify
> > -   it under the terms of the GNU Lesser General Public License as published
> > by
> > -   the Free Software Foundation; either version 2.1 of the License, or
> > +   it under the terms of the GNU General Public License as published by
> > +   the Free Software Foundation; either version 3 of the License, or
> > (at your option) any later version.
> >  
> > This program is distributed in the hope that it will be useful,
> > but WITHOUT ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > -   GNU Lesser General Public License for more details.
> > +   GNU General Public License for more details.
> >  
> 
> What ?? Sure about this?

Ah no definitely not, I forgot to run the magic gnulib command to adjust
the license. Will be fixed in v2.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-common 8/8] log: Let gcc know about the logging macros which abort

[Christophe Fergeau]

On Thu, Mar 28, 2019 at 02:02:58PM -0400, Frediano Ziglio wrote:
> > 
> > The for(;;) hack was taken from glib's logging macros.
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  common/log.h | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/common/log.h b/common/log.h
> > index 7c67e7a..b397306 100644
> > --- a/common/log.h
> > +++ b/common/log.h
> > @@ -20,6 +20,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -42,6 +43,7 @@ void spice_log(GLogLevelFlags log_level,
> >  #define spice_return_if_fail(x) G_STMT_START {  \
> >  if G_LIKELY(x) { } else {   \
> >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, G_STRFUNC, "condition
> >  `%s' failed", #x); \
> > +abort();
> > \
> >  return; \
> >  }   \
> >  } G_STMT_END
> > @@ -49,6 +51,7 @@ void spice_log(GLogLevelFlags log_level,
> >  #define spice_return_val_if_fail(x, val) G_STMT_START { \
> >  if G_LIKELY(x) { } else {   \
> >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__,
> >  "condition `%s' failed", #x); \
> > +abort();
> > \
> >  return (val);   \
> >  }   \
> >  } G_STMT_END
> > @@ -69,12 +72,17 @@ void spice_log(GLogLevelFlags log_level,
> >  spice_log(G_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, "" format, 
> > ##
> >  __VA_ARGS__); \
> >  } G_STMT_END
> >  
> > +/* for(;;) ; so that GCC knows that control doesn't go past g_error().
> 
> g_error? copy error?
> 
> > + * Put space before ending semicolon to avoid C++ build warnings.
> > + */
> >  #define spice_critical(format, ...) G_STMT_START {
> >  \
> >  spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format,
> >  ## __VA_ARGS__); \
> > +for (;;) ;
> 
> I suppose you can use "for(;;) continue;" and remove the comment (that
> "continue" was an old suggestion I had, I agree with C++ warning like
> I agreed at that time with the continue).
> 
> Why some are for and some abort?

Actually, I can't remember, and I haven't been able to reproduce the
warnings I wanted to fix. I changed all to abort(), which should be
enough.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-common] recorder: Update

[Frediano Ziglio]

Pull some fixes and features.
One of the feature is the support for @output setting to redirect
log output.

Signed-off-by: Frediano Ziglio 
---
 common/recorder | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/recorder b/common/recorder
index 03eb4b6..10b1787 16
--- a/common/recorder
+++ b/common/recorder
@@ -1 +1 @@
-Subproject commit 03eb4b6ef7aa90645a7395ea8bea55ebf33d6632
+Subproject commit 10b178762e1d66d82bdde754385a6a608a730607
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH linux/vd-agent 10/11] clipboard: only send release when no immediate grab

[Frediano Ziglio]

> 
> Hi
> 
> On Thu, Mar 28, 2019 at 5:30 PM Frediano Ziglio  wrote:
> >
> > >
> > > From: Marc-André Lureau 
> > >
> > > Do not send a release event between two grabs, this helps with window
> > > manager interaction issues on peer side.
> > >
> >
> > I would explain which kind of issue this is supposed to fix.
> 
> They react to clipboard events in different ways. The point is to
> minimize the amount of events to avoid extra unnecessary interactions.
> 
> In particular, on release event, some clipboard managers take the
> grab. This creates a race with Spice which does a release between
> grabs currently.
> 

As clipboard managers are not using SPICE I suppose here you
are talking about desktop clipboard release, not agent RELEASE
message. But on the previous sentence we were speaking about
agent RELEASE.

> >
> > > Advertise this behaviour via a capability introduced in spice-protocol
> > > 0.12.16, so the client doesn't need to do some time-based filtering.
> > >
> > > (the capability shouldn't need to be negotiated, a client shouldn't
> > > expect a release between two grabs)
> > >
> > > Signed-off-by: Marc-André Lureau 
> >
> > I suppose Jakub/Victor could do a much better review/test than me.
> >
> > > ---
> > >  src/vdagent/clipboard.c | 12 ++--
> > >  src/vdagent/x11.c   |  7 +++
> > >  src/vdagentd/vdagentd.c |  1 +
> > >  3 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/src/vdagent/clipboard.c b/src/vdagent/clipboard.c
> > > index 9fb87e3..097c6ee 100644
> > > --- a/src/vdagent/clipboard.c
> > > +++ b/src/vdagent/clipboard.c
> > > @@ -242,13 +242,13 @@ static void clipboard_owner_change_cb(GtkClipboard
> > > *clipboard,
> > >  return;
> > >  }
> > >
> > > -if (sel->owner == OWNER_GUEST) {
> > > -clipboard_new_owner(c, sel_id, OWNER_NONE);
> > > -udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0,
> > > NULL,
> > > 0);
> > > -}
> > > -
> > > -if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER)
> > > +if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
> > > +if (sel->owner == OWNER_GUEST) {
> > > +clipboard_new_owner(c, sel_id, OWNER_NONE);
> > > +udscs_write(c->conn, VDAGENTD_CLIPBOARD_RELEASE, sel_id, 0,
> > > NULL, 0);
> > > +}
> > >  return;
> > > +}
> > >
> > >  /* if there's a pending request for clipboard targets, cancel it */
> > >  if (sel->last_targets_req)
> > > diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> > > index c2515a8..9409b53 100644
> > > --- a/src/vdagent/x11.c
> > > +++ b/src/vdagent/x11.c
> > > @@ -530,11 +530,10 @@ static void vdagent_x11_handle_event(struct
> > > vdagent_x11
> > > *x11, XEvent event)
> > >  if (ev.xfev.owner == x11->selection_window)
> > >  return;
> > >
> > > -/* If the clipboard owner is changed we no longer own it */
> >
> > Why not keeping this comment?
> 
> The comment is no longer valid with this change: if the clipboard
> owner is changed, the agent may still own the grab (at the protocol
> level).
> 

Yes, this prove what I was saying in the previous e-mail, you are
mixing desktop ownership and SPICE grab, if you consider from
desktop prospective (which is using owner definition like in
"ev.xfev.owner") the comment is still valid, but you are reading
it with different definitions so it became invalid.

> As you can see with the following line being removed:
> >
> > > -vdagent_x11_set_clipboard_owner(x11, selection, owner_none);
> > > -
> > > -if (ev.xfev.owner == None)
> > > +if (ev.xfev.owner == None) {
> > > +vdagent_x11_set_clipboard_owner(x11, selection, owner_none);
> > >  return;
> > > +}
> > >
> > >  /* Request the supported targets from the new owner */
> > >  XConvertSelection(x11->display, ev.xfev.selection,
> > >  x11->targets_atom,
> > > diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> > > index 72a3e13..683e5d3 100644
> > > --- a/src/vdagentd/vdagentd.c
> > > +++ b/src/vdagentd/vdagentd.c
> > > @@ -133,6 +133,7 @@ static void send_capabilities(struct
> > > vdagent_virtio_port
> > > *vport,
> > >  VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MAX_CLIPBOARD);
> > >  VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_AUDIO_VOLUME_SYNC);
> > >  VD_AGENT_SET_CAPABILITY(caps->caps,
> > >  VD_AGENT_CAP_GRAPHICS_DEVICE_INFO);
> > > +VD_AGENT_SET_CAPABILITY(caps->caps,
> > > VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB);
> > >  virtio_msg_uint32_to_le((uint8_t *)caps, size, 0);
> > >
> > >  vdagent_virtio_port_write(vport, VDP_CLIENT_PORT,
> >
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL

[Frediano Ziglio]

> 
> Hi
> 
> On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio  wrote:
> >
> > >
> > > ..Hi
> > >
> > > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio 
> > > wrote:
> > > > > The role of the grab message is to take ownership of the clipboard
> > > > > (to
> > > > > advertize clipboard data available). It may come at any time from
> > > > > both
> > > > > side, and override the current grab owner. It may come from the guest
> > > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > > Or it may come from the client (C-c in client side app), and the
> > > > > agent
> > > > > will grab the guest clipboard.
> > > > >
> > > >
> > > > Yes, I realized this morning thinking about how clipboards works
> > > > (very rusty in my mind).
> > > > Instead of setting it you get the ownership that is you are willing
> > > > to set a value on the clipboard but you defer setting it to avoid
> > > > the expense of data copy.
> > > > So, the old (original?) protocol was simply to sending entire data
> > > > on every change, this avoided to loose the clipboard data entirely.
> > >
> > > I don't even remember that version. It looks like the original version
> > > was already "on-demand"
> > >
> > >
> > > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > > Author: Arnon Gilboa 
> > > Date:   Mon Oct 4 16:45:15 2010 +0200
> > >
> > > vd_agent: add protocol messages for clipboard/selection-owner model
> > >
> > > -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > > application in our side ("we") got ownership of the clipboard.
> > > -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > > owns the clipboard (GRAB received), we notify the os we are the owner.
> > > when we are asked by the os/app for the clipboard data, we send this
> > > RE
> > > QUEST msg to the other side.
> > > -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > > the clipboard, is now sent only in response to REQUEST.
> > > -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > > longer the owner of the clipboard (e.g. the owner app was closed).
> > >
> > > this patch will be followed by agent & client patches handling the
> > > above messages.
> > >
> > >
> > >
> > > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
> > >
> >
> > I suppose the "now" in "the existing message for sending the clipboard, is
> > now sent only in response to REQUEST" means that was changed.
> >
> > I personally think the code should be readable from a tarball/snapshot not
> > pretending people to dig into 10 years of history. I'll try to find some
> > time
> > to put these lines into vd_agent.h.
> >
> > > > The current is: if we get new local clipboard data send the "grab"
> > > > so remote knows that will have to read our data.
> > >
> > > yes
> > >
> > > > So either we have ownership/grab, meaning the data are remote
> > > > waiting to get fetched or we don't have ownership and the remote
> > > > should be grabbing as we have data to send (at least in a stable
> > > > state).
> > >
> > >
> > > That last sentence is confusing. There are 2 states the client can
> > > "have the grab".
> > >
> > > 1. the client took the grab for the remote data: we are talking about
> > > the client app in the client desktop environment
> > > 2. the client took the grab, at the protocol level: it sent a grab
> > > over the protocol to announce it can provide clipboard data to the
> > > remote. In this case, the client app doesn't have the grab in the
> > > client desktop environment, but another client application.
> > >
> > > In general, when talking about the protocol, "client has the grab"
> > > means 2) to me, iow it can provide data to the remote.
> > >
> >
> > I think I mean the opposite. The reason is that some application
> > have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> > the current local application (so spice-gtk/vd_agent) does NOT have the
> > ownership and it's asking the remote to take to the application (so will
> > remove the ownership from another remote application).
> > From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> > other side to grab (take ownership) of the clipboard so will be the
> > remote to have the "grab", not the local.
> 
> 
> I find it hard to understand you. The sequence of events is as such.
> 
> A & B are client or remote exchangeably:
> - an application on A takes the clipboard grab (== announce clipboard
> data is available)
> - A get notified by the desktop and sends a GRAB message to B side
> - B receives GRAB, and takes the clipboard grab on B desktop side
> 
> With this sequence, "A" has the clipboard grab at the Spice protocol
> level, so to say. It means there is clipboard data on A side.
> 

Not clear either. If what you wrote it's correct, what I read:
You wrote "B receives GRAB, and takes the clipboard grab" so I suppose
that B now have the "clipboard grab", however 

[Spice-devel] [PATCH spice-server v5 18/18] Add some notes for the Windows port

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 README.Windows | 18 ++
 1 file changed, 18 insertions(+)
 create mode 100644 README.Windows

diff --git a/README.Windows b/README.Windows
new file mode 100644
index 0..a953813de
--- /dev/null
+++ b/README.Windows
@@ -0,0 +1,18 @@
+SPICE server Windows support
+
+
+SPICE server was ported from Unix/Linux to Windows.
+
+Most features are present, with some exceptions:
+- Unix sockets;
+- signal handling;
+- CLOEXEC flag (completely different handling on Windows);
+- IPTOS_LOWDELAY flag (Linux specific);
+- TCP_CORK (Linux/*BSD specific).
+
+Some features could be ported but currently are not:
+- statistics exported through mapped files. Disabled by default and mainly
+  used for development;
+- filtering while recording (SPICE_WORKER_RECORD_FILTER environment).
+  Recording is used for debugging or development work;
+- TCP_KEEPIDLE setting. Default is used.
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 05/18] sys-socket: Introduce some utility to make sockets more portable

[Frediano Ziglio]

Between Unix and Windows socket are quite different:
- on Windows sockets have a different namespace from C file
  descriptors so you can't use read/write/close or similar functions;
- errors are not stored in errno but you must be read/write the
  errors with specific function;
- sometimes sockets are put in non-blocking mode automatically
  calling some functions;
- SOCKET type is 64 bit on Windows 64 which does not fit technically
  in an int. Is however safe to assume them to fit in an int.

So encapsulate the socket APIs in some definition to make easier
and more safe to deal with them.
Where the portability to Windows would make to code more offuscated a Unix
style was preferred. For instance if errors are detected errno is set from
Windows socket error instead of changing all code handling.
Fortunately on Windows Qemu core interface accepts socket (but not
other types like C file descriptors!).

Signed-off-by: Frediano Ziglio 
Reviewed-by: Marc-André Lureau 
---
 server/Makefile.am  |   2 +
 server/meson.build  |   2 +
 server/sys-socket.c | 212 
 server/sys-socket.h | 139 +
 4 files changed, 355 insertions(+)
 create mode 100644 server/sys-socket.c
 create mode 100644 server/sys-socket.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 34ec22ad5..7f2606129 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -166,6 +166,8 @@ libserver_la_SOURCES =  \
stat.h  \
stream-channel.c\
stream-channel.h\
+   sys-socket.h\
+   sys-socket.c\
red-stream-device.c \
red-stream-device.h \
sw-canvas.c \
diff --git a/server/meson.build b/server/meson.build
index 63191d792..34d8eef18 100644
--- a/server/meson.build
+++ b/server/meson.build
@@ -133,6 +133,8 @@ spice_server_sources = [
   'stat.h',
   'stream-channel.c',
   'stream-channel.h',
+  'sys-socket.c',
+  'sys-socket.h',
   'red-stream-device.c',
   'red-stream-device.h',
   'sw-canvas.c',
diff --git a/server/sys-socket.c b/server/sys-socket.c
new file mode 100644
index 0..7ce5dab1a
--- /dev/null
+++ b/server/sys-socket.c
@@ -0,0 +1,212 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2018 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#ifndef _WIN32
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
+
+#include 
+
+#include "sys-socket.h"
+
+#ifdef _WIN32
+// Map Windows socket errors to standard C ones
+// See 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms740668(v=vs.85).aspx
+void socket_win32_set_errno(void)
+{
+int err = EPIPE; // default
+switch (WSAGetLastError()) {
+case WSAEWOULDBLOCK:
+case WSAEINPROGRESS:
+err = EAGAIN;
+break;
+case WSAEINTR:
+err = EINTR;
+break;
+case WSAEBADF:
+err = EBADF;
+break;
+case WSA_INVALID_HANDLE:
+case WSA_INVALID_PARAMETER:
+case WSAEINVAL:
+err = EINVAL;
+break;
+case WSAENOTSOCK:
+err = ENOTSOCK;
+break;
+case WSA_NOT_ENOUGH_MEMORY:
+err = ENOMEM;
+break;
+case WSAEPROTONOSUPPORT:
+case WSAESOCKTNOSUPPORT:
+case WSAEOPNOTSUPP:
+case WSAEPFNOSUPPORT:
+case WSAEAFNOSUPPORT:
+case WSAVERNOTSUPPORTED:
+err = ENOTSUP;
+break;
+case WSAEFAULT:
+err = EFAULT;
+break;
+case WSAEACCES:
+err = EACCES;
+break;
+case WSAEMFILE:
+err = EMFILE;
+break;
+case WSAENAMETOOLONG:
+err = ENAMETOOLONG;
+break;
+case WSAENOTEMPTY:
+err = ENOTEMPTY;
+break;
+case WSA_OPERATION_ABORTED:
+case WSAECANCELLED:
+case WSA_E_CANCELLED:
+err = ECANCELED;
+break;
+case WSAEADDRINUSE:
+err = EADDRINUSE;
+break;
+case WSAENETDOWN:
+err = ENETDOWN;
+break;
+case WSAENETUNREACH:
+err = ENETUNREACH;
+break;
+  

[Spice-devel] [PATCH spice-server v5 17/18] Disable recording filtering for Windows

[Frediano Ziglio]

Although this feature can be ported to Windows doing so would
require the usage of g_spawn_async_with_fds, which is only available
in GLib 2.58 or some specific Win32 code.

Signed-off-by: Frediano Ziglio 
---
 server/red-record-qxl.c| 7 +++
 server/tests/test-record.c | 7 +--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index 30a3b0dae..f3a2dc391 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -817,6 +817,7 @@ void red_record_qxl_command(RedRecord *record, 
RedMemSlotInfo *slots,
 pthread_mutex_unlock(>lock);
 }
 
+#ifndef _WIN32
 /**
  * Redirects child output to the file specified
  */
@@ -829,6 +830,7 @@ static void child_output_setup(gpointer user_data)
 }
 close(fd);
 }
+#endif
 
 RedRecord *red_record_new(const char *filename)
 {
@@ -845,6 +847,7 @@ RedRecord *red_record_new(const char *filename)
 
 filter = getenv("SPICE_WORKER_RECORD_FILTER");
 if (filter) {
+#ifndef _WIN32
 gint argc;
 gchar **argv = NULL;
 GError *error = NULL;
@@ -870,6 +873,10 @@ RedRecord *red_record_new(const char *filename)
 }
 close(fd_in);
 g_spawn_close_pid(child_pid);
+#else
+// TODO
+spice_warning("recorder filter not supported under Windows");
+#endif
 }
 
 if (fwrite(header, sizeof(header)-1, 1, f) != 1) {
diff --git a/server/tests/test-record.c b/server/tests/test-record.c
index de3c6f5bd..8ee36cebd 100644
--- a/server/tests/test-record.c
+++ b/server/tests/test-record.c
@@ -35,9 +35,9 @@ test_record(bool compress)
 RedRecord *rec;
 const char *fn = OUTPUT_FILENAME;
 
-unsetenv("SPICE_WORKER_RECORD_FILTER");
+g_unsetenv("SPICE_WORKER_RECORD_FILTER");
 if (compress) {
-setenv("SPICE_WORKER_RECORD_FILTER", "gzip", 1);
+g_setenv("SPICE_WORKER_RECORD_FILTER", "gzip", 1);
 }
 
 // delete possible stale test output
@@ -95,6 +95,9 @@ int
 main(void)
 {
 test_record(false);
+// TODO implement on Windows
+#ifndef _WIN32
 test_record(true);
+#endif
 return 0;
 }
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 06/18] sys-socket: Add socket_newpair utility

[Frediano Ziglio]

Allows to easier port socketpair.
Windows does not have this function, we need to create a pair
using 2 internet sockets and connecting one to the other.
The SPICE core interface implementation provided by Qemu under
Windows requires, under Windows, to provide SOCKET handles
so pipes or other Windows handles won't work.
Windows does not provide a socketpair function so use this
replacement.

Signed-off-by: Frediano Ziglio 
---
 server/sys-socket.c | 75 +
 server/sys-socket.h |  3 ++
 2 files changed, 78 insertions(+)

diff --git a/server/sys-socket.c b/server/sys-socket.c
index 7ce5dab1a..837da18e3 100644
--- a/server/sys-socket.c
+++ b/server/sys-socket.c
@@ -209,4 +209,79 @@ SPICE_CONSTRUCTOR_FUNC(socket_win32_init)
 WSADATA wsaData;
 WSAStartup(MAKEWORD(2, 2), );
 }
+
+int socket_newpair(int type, int protocol, int sv[2])
+{
+struct sockaddr_in sa, sa2;
+socklen_t addrlen;
+SOCKET s, pairs[2];
+
+if (!sv) {
+return -1;
+}
+
+/* create a listener */
+s = socket(AF_INET, type, 0);
+if (s == INVALID_SOCKET) {
+return -1;
+}
+
+pairs[1] = INVALID_SOCKET;
+
+pairs[0] = socket(AF_INET, type, 0);
+if (pairs[0] == INVALID_SOCKET) {
+goto cleanup;
+}
+
+/* bind to a random port */
+sa.sin_family = AF_INET;
+sa.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+sa.sin_port = 0;
+if (bind(s, (struct sockaddr*) , sizeof(sa)) < 0) {
+goto cleanup;
+}
+if (listen(s, 1) < 0) {
+goto cleanup;
+}
+
+/* connect to kernel choosen port */
+addrlen = sizeof(sa);
+if (getsockname(s, (struct sockaddr*) , ) < 0) {
+goto cleanup;
+}
+if (connect(pairs[0], (struct sockaddr*) , sizeof(sa)) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+pairs[1] = accept(s, (struct sockaddr*) , );
+if (pairs[1] == INVALID_SOCKET) {
+goto cleanup;
+}
+
+/* check proper connection */
+addrlen = sizeof(sa);
+if (getsockname(pairs[0], (struct sockaddr*) , ) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+if (getpeername(pairs[1], (struct sockaddr*) , ) < 0) {
+goto cleanup;
+}
+if (sa.sin_family != sa2.sin_family || sa.sin_port != sa2.sin_port
+|| sa.sin_addr.s_addr != sa2.sin_addr.s_addr) {
+goto cleanup;
+}
+
+closesocket(s);
+sv[0] = pairs[0];
+sv[1] = pairs[1];
+return 0;
+
+cleanup:
+socket_win32_set_errno();
+closesocket(s);
+closesocket(pairs[0]);
+closesocket(pairs[1]);
+return -1;
+}
 #endif
diff --git a/server/sys-socket.h b/server/sys-socket.h
index 650625711..3a3b78789 100644
--- a/server/sys-socket.h
+++ b/server/sys-socket.h
@@ -134,6 +134,9 @@ socket_accept(int sock, struct sockaddr *addr, int *addrlen)
 }
 #undef accept
 #define accept socket_accept
+
+int socket_newpair(int type, int protocol, int sv[2]);
+#define socketpair(family, type, protocol, sv) socket_newpair(type, protocol, 
sv)
 #endif
 
 #endif // RED_SYS_SOCKET_H_
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 15/18] tests: Exclude tests that cannot work on Windows

[Frediano Ziglio]

test-stream test is passing file descriptor using Unix socket.
test-stat-file needs some porting work of mmap feature.

Signed-off-by: Frediano Ziglio 
---
 server/tests/Makefile.am | 9 +++--
 server/tests/meson.build | 9 +++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index 7668739ff..c50826e62 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -53,11 +53,9 @@ check_PROGRAMS = \
test-codecs-parsing \
test-options\
test-stat   \
-   test-stream \
test-agent-msg-filter   \
test-loop   \
test-qxl-parsing\
-   test-stat-file  \
test-leaks  \
test-vdagent\
test-fail-on-null-core-interface\
@@ -68,6 +66,13 @@ check_PROGRAMS = \
test-record \
$(NULL)
 
+if !OS_WIN32
+check_PROGRAMS +=  \
+   test-stream \
+   test-stat-file  \
+   $(NULL)
+endif
+
 noinst_PROGRAMS =  \
test-display-no-ssl \
test-display-streaming  \
diff --git a/server/tests/meson.build b/server/tests/meson.build
index 31febc6ac..b4269c58b 100644
--- a/server/tests/meson.build
+++ b/server/tests/meson.build
@@ -40,11 +40,9 @@ tests = [
   ['test-codecs-parsing', true],
   ['test-options', true],
   ['test-stat', true],
-  ['test-stream', true],
   ['test-agent-msg-filter', true],
   ['test-loop', true],
   ['test-qxl-parsing', true],
-  ['test-stat-file', true],
   ['test-leaks', true],
   ['test-vdagent', true],
   ['test-fail-on-null-core-interface', true],
@@ -65,6 +63,13 @@ if spice_server_has_sasl
   tests += [['test-sasl', true]]
 endif
 
+if host_machine.system() != 'windows'
+  tests += [
+['test-stream', true],
+['test-stat-file', true],
+  ]
+endif
+
 if spice_server_has_gstreamer
   tests += [['test-gst', false]]
   if get_option('extra-checks')
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 09/18] red-stream: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-stream.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index 4fb2d2bf6..ab3d87c7c 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -114,7 +114,7 @@ static int socket_set_cork(int socket, int enabled)
 
 static ssize_t stream_write_cb(RedStream *s, const void *buf, size_t size)
 {
-return write(s->socket, buf, size);
+return socket_write(s->socket, buf, size);
 }
 
 static ssize_t stream_writev_cb(RedStream *s, const struct iovec *iov, int 
iovcnt)
@@ -132,7 +132,7 @@ static ssize_t stream_writev_cb(RedStream *s, const struct 
iovec *iov, int iovcn
 for (i = 0; i < tosend; i++) {
 expected += iov[i].iov_len;
 }
-n = writev(s->socket, iov, tosend);
+n = socket_writev(s->socket, iov, tosend);
 if (n <= expected) {
 if (n > 0)
 ret += n;
@@ -148,7 +148,7 @@ static ssize_t stream_writev_cb(RedStream *s, const struct 
iovec *iov, int iovcn
 
 static ssize_t stream_read_cb(RedStream *s, void *buf, size_t size)
 {
-return read(s->socket, buf, size);
+return socket_read(s->socket, buf, size);
 }
 
 static ssize_t stream_ssl_write_cb(RedStream *s, const void *buf, size_t size)
@@ -404,7 +404,7 @@ void red_stream_free(RedStream *s)
 }
 
 red_stream_remove_watch(s);
-close(s->socket);
+socket_close(s->socket);
 
 g_free(s);
 }
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 01/18] build: Detect Windows build and change some definitions

[Frediano Ziglio]

Windows needs some specific setting to use network.

Signed-off-by: Frediano Ziglio 
---
 configure.ac | 20 +++-
 meson.build  | 15 ---
 server/tests/meson.build |  5 -
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 604a41b21..f8b41f377 100644
--- a/configure.ac
+++ b/configure.ac
@@ -68,6 +68,20 @@ case $host_cpu in
 SPICE_WARNING([spice-server on non-x86_64 architectures has not been 
extensively tested])
 esac
 
+AC_MSG_CHECKING([for native Win32])
+case "$host_os" in
+ *mingw*|*cygwin*)
+os_win32=yes
+dnl AI_ADDRCONFIG and possibly some other code require at least Vista
+AC_DEFINE([_WIN32_WINNT], [0x600], [Minimal Win32 version])]
+;;
+ *)
+os_win32=no
+;;
+esac
+AC_MSG_RESULT([$os_win32])
+AM_CONDITIONAL([OS_WIN32],[test "$os_win32" = "yes"])
+
 dnl =
 dnl Check optional features
 SPICE_CHECK_SMARTCARD
@@ -154,6 +168,9 @@ AC_CHECK_LIB(rt, clock_gettime, LIBRT="-lrt")
 AC_SUBST(LIBRT)
 
 AS_VAR_APPEND([SPICE_NONPKGCONFIG_LIBS], [" -pthread $LIBM $LIBRT"])
+AS_IF([test "x$os_win32" = "xyes"], [
+AS_VAR_APPEND([SPICE_NONPKGCONFIG_LIBS], [" -lws2_32"])
+])
 
 SPICE_REQUIRES=""
 
@@ -176,7 +193,8 @@ PKG_CHECK_MODULES([GOBJECT2], [gobject-2.0 >= 
$GLIB2_REQUIRED])
 AS_VAR_APPEND([SPICE_REQUIRES], [" gobject-2.0 >= $GLIB2_REQUIRED"])
 
 #used only by tests
-PKG_CHECK_MODULES([GIO_UNIX], [gio-unix-2.0 >= $GLIB2_REQUIRED])
+AS_IF([test "x$os_win32" != "xyes"],
+  PKG_CHECK_MODULES([GIO_UNIX], [gio-unix-2.0 >= $GLIB2_REQUIRED]))
 
 PIXMAN_REQUIRED=0.17.7
 PKG_CHECK_MODULES(PIXMAN, pixman-1 >= $PIXMAN_REQUIRED)
diff --git a/meson.build b/meson.build
index 1b07f30d5..e56adc387 100644
--- a/meson.build
+++ b/meson.build
@@ -103,9 +103,13 @@ foreach dep : ['libjpeg', 'zlib']
   spice_server_deps += dependency(dep)
 endforeach
 
-foreach dep : ['librt', 'libm']
-  spice_server_deps += compiler.find_library(dep)
-endforeach
+if host_machine.system() != 'windows'
+  foreach dep : ['librt', 'libm']
+spice_server_deps += compiler.find_library(dep)
+  endforeach
+else
+  spice_server_deps += compiler.find_library('ws2_32')
+endif
 
 #
 # Non-mandatory/optional dependencies
@@ -202,6 +206,11 @@ if get_option('statistics')
   spice_server_config_data.set('RED_STATISTICS', '1')
 endif
 
+# Minimal Win32 version
+if host_machine.system() == 'windows'
+  spice_server_config_data.set('_WIN32_WINNT', '0x600')
+endif
+
 configure_file(output : 'config.h',
install : false,
configuration : spice_server_config_data)
diff --git a/server/tests/meson.build b/server/tests/meson.build
index b79b11080..205482130 100644
--- a/server/tests/meson.build
+++ b/server/tests/meson.build
@@ -1,5 +1,8 @@
 test_lib_include = [spice_server_include, include_directories('.')]
-test_lib_deps = [spice_server_deps, dependency('gio-unix-2.0')]
+test_lib_deps = [spice_server_deps]
+if host_machine.system() != 'windows'
+  test_lib_deps += [dependency('gio-unix-2.0')]
+endif
 
 test_lib_sources = [
   'basic-event-loop.c',
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 07/18] net-utils: Port to Windows

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/net-utils.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/server/net-utils.c b/server/net-utils.c
index 802509a49..3034dc5e1 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -35,6 +35,7 @@
 #include 
 
 #include "net-utils.h"
+#include "sys-socket.h"
 
 /**
  * red_socket_set_keepalive:
@@ -101,6 +102,15 @@ bool red_socket_set_no_delay(int fd, bool no_delay)
  */
 bool red_socket_set_non_blocking(int fd, bool non_blocking)
 {
+#ifdef _WIN32
+u_long ioctl_nonblocking = 1;
+
+if (ioctlsocket(fd, FIONBIO, _nonblocking) != 0) {
+spice_warning("ioctlsocket(FIONBIO) failed, %d", WSAGetLastError());
+return false;
+}
+return true;
+#else
 int flags;
 
 if ((flags = fcntl(fd, F_GETFL)) == -1) {
@@ -120,6 +130,7 @@ bool red_socket_set_non_blocking(int fd, bool non_blocking)
 }
 
 return true;
+#endif
 }
 
 /**
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 02/18] Avoids %m in formatting for Windows

[Frediano Ziglio]

Not supported, %m is a GNU extension of sscanf.

Signed-off-by: Frediano Ziglio 
---
 server/reds.c | 29 ++---
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 28542bd09..302da5858 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3741,8 +3741,7 @@ static const int video_codec_caps[] = {
  * @codec: a location to return the parsed codec
  * @return the position of the next codec in the string
  */
-static const char* parse_next_video_codec(const char *codecs, char **encoder,
-  char **codec)
+static char* parse_next_video_codec(char *codecs, char **encoder, char **codec)
 {
 if (!codecs) {
 return NULL;
@@ -3751,14 +3750,15 @@ static const char* parse_next_video_codec(const char 
*codecs, char **encoder,
 if (!*codecs) {
 return NULL;
 }
-int n;
+int end_encoder, end_codec = -1;
 *encoder = *codec = NULL;
-if (sscanf(codecs, "%m[0-9a-zA-Z_]:%m[0-9a-zA-Z_]%n", encoder, codec, ) 
== 2) {
-// this avoids accepting "encoder:codec" followed by garbage like "$%*"
-if (codecs[n] != ';' && codecs[n] != '\0') {
-free(*codec);
-*codec = NULL;
-}
+if (sscanf(codecs, "%*[0-9a-zA-Z_]:%n%*[0-9a-zA-Z_];%n", _encoder, 
_codec) == 0
+&& end_codec > 0) {
+codecs[end_encoder - 1] = '\0';
+codecs[end_codec - 1] = '\0';
+*encoder = codecs;
+*codec = codecs + end_encoder;
+return codecs + end_codec;
 }
 return codecs + strcspn(codecs, ";");
 }
@@ -3775,7 +3775,8 @@ static void reds_set_video_codecs_from_string(RedsState 
*reds, const char *codec
 }
 
 video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
-const char *c = codecs;
+char *codecs_copy = g_strdup_printf("%s;", codecs);
+char *c = codecs_copy;
 while ( (c = parse_next_video_codec(c, _name, _name)) ) {
 uint32_t encoder_index, codec_index;
 if (!encoder_name || !codec_name) {
@@ -3798,19 +3799,17 @@ static void reds_set_video_codecs_from_string(RedsState 
*reds, const char *codec
 g_array_append_val(video_codecs, new_codec);
 }
 
-/* these are allocated by sscanf, do not use g_free */
-free(encoder_name);
-free(codec_name);
 codecs = c;
 }
 
 if (video_codecs->len == 0) {
 spice_warning("Failed to set video codecs, input string: '%s'", 
codecs);
 g_array_unref(video_codecs);
-return;
+} else {
+reds_set_video_codecs(reds, video_codecs);
 }
 
-reds_set_video_codecs(reds, video_codecs);
+g_free(codecs_copy);
 }
 
 SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer *reds, SpiceCoreInterface 
*core)
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 11/18] test-leaks: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/tests/test-leaks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/server/tests/test-leaks.c b/server/tests/test-leaks.c
index 64130c22a..be9fe2d2e 100644
--- a/server/tests/test-leaks.c
+++ b/server/tests/test-leaks.c
@@ -35,6 +35,7 @@
 #include "test-glib-compat.h"
 #include "basic-event-loop.h"
 #include "test-display-base.h"
+#include "sys-socket.h"
 
 #define PKI_DIR SPICE_TOP_SRCDIR "/server/tests/pki/"
 
@@ -70,11 +71,11 @@ static void server_leaks(void)
 g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
   "*SSL_accept failed*");
 g_assert_cmpint(socketpair(AF_LOCAL, SOCK_STREAM, 0, sv), ==, 0);
-close(sv[1]);
+socket_close(sv[1]);
 result = spice_server_add_ssl_client(server, sv[0], 1);
 g_assert_cmpint(result, ==, -1);
 /* if the function fails, it should not close the socket */
-g_assert_cmpint(close(sv[0]), ==, 0);
+g_assert_cmpint(socket_close(sv[0]), ==, 0);
 
 spice_server_destroy(server);
 basic_event_loop_destroy();
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 08/18] reds: Use socket compatibility layer (close -> socket_close)

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/red-common.h |  1 +
 server/reds.c   | 11 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/server/red-common.h b/server/red-common.h
index 181ed283c..6b5d0b2e0 100644
--- a/server/red-common.h
+++ b/server/red-common.h
@@ -35,6 +35,7 @@
 
 #include "spice.h"
 #include "utils.h"
+#include "sys-socket.h"
 
 #define SPICE_UPCAST(type, ptr) \
 (verify_expr(SPICE_OFFSETOF(type, base) == 0,SPICE_CONTAINEROF(ptr, type, 
base)))
diff --git a/server/reds.c b/server/reds.c
index 0aae3589f..71a03bd0b 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -108,6 +108,7 @@ static void adapter_timer_remove(const 
SpiceCoreInterfaceInternal *iface, SpiceT
 static SpiceWatch *adapter_watch_add(const SpiceCoreInterfaceInternal *iface,
  int fd, int event_mask, SpiceWatchFunc 
func, void *opaque)
 {
+// note: Qemu API is fine having a SOCKET on Windows
 return iface->public_interface->watch_add(fd, event_mask, func, opaque);
 }
 
@@ -2661,7 +2662,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 len = SUN_LEN();
 if (bind(slisten, (struct sockaddr *), len) == -1) {
 perror("bind");
-close(slisten);
+socket_close(slisten);
 return -1;
 }
 
@@ -2709,7 +2710,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 freeaddrinfo(res);
 goto listen;
 }
-close(slisten);
+socket_close(slisten);
 }
 spice_warning("binding socket to %s:%d failed", addr, portnr);
 freeaddrinfo(res);
@@ -2718,7 +2719,7 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 listen:
 if (listen(slisten, SOMAXCONN) != 0) {
 spice_warning("listen: %s", strerror(errno));
-close(slisten);
+socket_close(slisten);
 return -1;
 }
 return slisten;
@@ -2756,14 +2757,14 @@ static void reds_cleanup_net(SpiceServer *reds)
 if (reds->listen_socket != -1) {
reds_core_watch_remove(reds, reds->listen_watch);
if (reds->config->spice_listen_socket_fd != reds->listen_socket) {
-  close(reds->listen_socket);
+  socket_close(reds->listen_socket);
}
reds->listen_watch = NULL;
reds->listen_socket = -1;
 }
 if (reds->secure_listen_socket != -1) {
reds_core_watch_remove(reds, reds->secure_listen_watch);
-   close(reds->secure_listen_socket);
+   socket_close(reds->secure_listen_socket);
reds->secure_listen_watch = NULL;
reds->secure_listen_socket = -1;
 }
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 14/18] dispatcher: Port to Windows

[Frediano Ziglio]

Replace poll call with select.
As socket is set to non-blocking we must support it so if
we detect an EAGAIN error wait for data.

Signed-off-by: Frediano Ziglio 
---
 server/dispatcher.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index 9a49a9899..ae505efae 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -201,6 +201,7 @@ static int read_safe(int fd, uint8_t *buf, size_t size, int 
block)
 }
 
 if (!block) {
+#ifndef _WIN32
 struct pollfd pollfd = {.fd = fd, .events = POLLIN, .revents = 0};
 while ((ret = poll(, 1, 0)) == -1) {
 if (errno == EINTR) {
@@ -213,6 +214,15 @@ static int read_safe(int fd, uint8_t *buf, size_t size, 
int block)
 if (!(pollfd.revents & POLLIN)) {
 return 0;
 }
+#else
+struct timeval tv = { 0, 0 };
+fd_set fds;
+FD_ZERO();
+FD_SET(fd, );
+if (select(1, , NULL, NULL, ) < 1) {
+return 0;
+}
+#endif
 }
 while (read_size < size) {
 ret = socket_read(fd, buf + read_size, size - read_size);
@@ -221,6 +231,16 @@ static int read_safe(int fd, uint8_t *buf, size_t size, 
int block)
 spice_debug("EINTR in read");
 continue;
 }
+#ifdef _WIN32
+// Windows turns this socket not-blocking
+if (errno == EAGAIN) {
+fd_set fds;
+FD_ZERO();
+FD_SET(fd, );
+select(1, , NULL, NULL, NULL);
+continue;
+}
+#endif
 return -1;
 }
 if (ret == 0) {
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 10/18] dispatcher: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/dispatcher.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index bae73f7db..9a49a9899 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -115,8 +115,8 @@ dispatcher_finalize(GObject *object)
 {
 Dispatcher *self = DISPATCHER(object);
 g_free(self->priv->messages);
-close(self->priv->send_fd);
-close(self->priv->recv_fd);
+socket_close(self->priv->send_fd);
+socket_close(self->priv->recv_fd);
 pthread_mutex_destroy(>priv->lock);
 g_free(self->priv->payload);
 G_OBJECT_CLASS(dispatcher_parent_class)->finalize(object);
@@ -215,7 +215,7 @@ static int read_safe(int fd, uint8_t *buf, size_t size, int 
block)
 }
 }
 while (read_size < size) {
-ret = read(fd, buf + read_size, size - read_size);
+ret = socket_read(fd, buf + read_size, size - read_size);
 if (ret == -1) {
 if (errno == EINTR) {
 spice_debug("EINTR in read");
@@ -242,7 +242,7 @@ static int write_safe(int fd, uint8_t *buf, size_t size)
 int ret;
 
 while (written_size < size) {
-ret = write(fd, buf + written_size, size - written_size);
+ret = socket_write(fd, buf + written_size, size - written_size);
 if (ret == -1) {
 if (errno != EINTR) {
 return -1;
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 16/18] red-stream: Fix SSL connection for Windows

[Frediano Ziglio]

Set correctly errno to make callers handle correctly encrypted
traffic.

Signed-off-by: Frediano Ziglio 
---
 server/red-stream.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index e8e88fb39..87939dc9a 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -158,15 +158,37 @@ static ssize_t stream_read_cb(RedStream *s, void *buf, 
size_t size)
 return socket_read(s->socket, buf, size);
 }
 
+static ssize_t stream_ssl_error(RedStream *s, int return_code)
+{
+SPICE_GNUC_UNUSED int ssl_error;
+
+ssl_error = SSL_get_error(s->priv->ssl, return_code);
+
+// OpenSSL can to return SSL_ERROR_WANT_READ if we attempt to read
+// data and the socket did not receive all SSL packet.
+// Under Windows errno is not set so potentially caller can detect
+// the wrong error so we need to set errno.
+#ifdef _WIN32
+if (ssl_error == SSL_ERROR_WANT_READ || ssl_error == SSL_ERROR_WANT_WRITE) 
{
+errno = EAGAIN;
+} else {
+errno = EPIPE;
+}
+#endif
+
+// red_peer_receive is expected to receive -1 on errors while
+// OpenSSL documentation just state a <0 value
+return -1;
+}
+
 static ssize_t stream_ssl_write_cb(RedStream *s, const void *buf, size_t size)
 {
 int return_code;
-SPICE_GNUC_UNUSED int ssl_error;
 
 return_code = SSL_write(s->priv->ssl, buf, size);
 
 if (return_code < 0) {
-ssl_error = SSL_get_error(s->priv->ssl, return_code);
+return stream_ssl_error(s, return_code);
 }
 
 return return_code;
@@ -175,12 +197,11 @@ static ssize_t stream_ssl_write_cb(RedStream *s, const 
void *buf, size_t size)
 static ssize_t stream_ssl_read_cb(RedStream *s, void *buf, size_t size)
 {
 int return_code;
-SPICE_GNUC_UNUSED int ssl_error;
 
 return_code = SSL_read(s->priv->ssl, buf, size);
 
 if (return_code < 0) {
-ssl_error = SSL_get_error(s->priv->ssl, return_code);
+return stream_ssl_error(s, return_code);
 }
 
 return return_code;
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 04/18] tests: Provide alarm replacement for Windows

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/tests/Makefile.am  |  2 +
 server/tests/meson.build  |  2 +
 server/tests/test-channel.c   |  1 +
 server/tests/test-loop.c  |  1 +
 server/tests/test-stream-device.c |  1 +
 server/tests/win-alarm.c  | 65 +++
 server/tests/win-alarm.h  | 26 +
 7 files changed, 98 insertions(+)
 create mode 100644 server/tests/win-alarm.c
 create mode 100644 server/tests/win-alarm.h

diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index d7f7af9ba..7668739ff 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -35,6 +35,8 @@ libtest_a_SOURCES =   \
test-display-base.h \
test-glib-compat.c  \
test-glib-compat.h  \
+   win-alarm.c \
+   win-alarm.h \
$(NULL)
 
 LDADD =\
diff --git a/server/tests/meson.build b/server/tests/meson.build
index 205482130..31febc6ac 100644
--- a/server/tests/meson.build
+++ b/server/tests/meson.build
@@ -11,6 +11,8 @@ test_lib_sources = [
   'test-display-base.h',
   'test-glib-compat.c',
   'test-glib-compat.h',
+  'win-alarm.c',
+  'win-alarm.h',
 ]
 
 test_libs = []
diff --git a/server/tests/test-channel.c b/server/tests/test-channel.c
index 9700e31c1..fcea98aa2 100644
--- a/server/tests/test-channel.c
+++ b/server/tests/test-channel.c
@@ -28,6 +28,7 @@
 #include "red-client.h"
 #include "cursor-channel.h"
 #include "net-utils.h"
+#include "win-alarm.h"
 
 /*
  * Declare a RedTestChannel to be used for the test
diff --git a/server/tests/test-loop.c b/server/tests/test-loop.c
index 1e3b39e53..82af80ab3 100644
--- a/server/tests/test-loop.c
+++ b/server/tests/test-loop.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include "basic-event-loop.h"
+#include "win-alarm.h"
 
 static SpiceCoreInterface *core = NULL;
 static GMainLoop *loop = NULL;
diff --git a/server/tests/test-stream-device.c 
b/server/tests/test-stream-device.c
index ce37822f7..f5698c7d5 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -33,6 +33,7 @@
 #include "test-glib-compat.h"
 #include "stream-channel.h"
 #include "reds.h"
+#include "win-alarm.h"
 
 static SpiceCharDeviceInstance vmc_instance;
 
diff --git a/server/tests/win-alarm.c b/server/tests/win-alarm.c
new file mode 100644
index 0..225d07097
--- /dev/null
+++ b/server/tests/win-alarm.c
@@ -0,0 +1,65 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2018 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+
+#include 
+#include 
+
+#ifdef _WIN32
+#include 
+#include "win-alarm.h"
+
+static HANDLE alarm_cond = NULL;
+
+static DWORD WINAPI alarm_thread_proc(LPVOID arg)
+{
+unsigned int timeout = (uintptr_t) arg;
+switch (WaitForSingleObject(alarm_cond, timeout * 1000)) {
+case WAIT_OBJECT_0:
+return 0;
+}
+abort();
+return 0;
+}
+
+void alarm(unsigned int timeout)
+{
+static HANDLE thread = NULL;
+
+// create an event to stop the alarm thread
+if (alarm_cond == NULL) {
+alarm_cond = CreateEvent(NULL, TRUE, FALSE, NULL);
+g_assert(alarm_cond != NULL);
+}
+
+// stop old alarm
+if (thread) {
+SetEvent(alarm_cond);
+g_assert(WaitForSingleObject(thread, INFINITE) == WAIT_OBJECT_0);
+CloseHandle(thread);
+thread = NULL;
+}
+
+if (timeout) {
+ResetEvent(alarm_cond);
+
+// start alarm thread
+thread = CreateThread(NULL, 0, alarm_thread_proc, (LPVOID) (uintptr_t) 
timeout, 0, NULL);
+g_assert(thread);
+}
+}
+#endif
diff --git a/server/tests/win-alarm.h b/server/tests/win-alarm.h
new file mode 100644
index 0..a7233a8f7
--- /dev/null
+++ b/server/tests/win-alarm.h
@@ -0,0 +1,26 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2018 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at 

[Spice-devel] [PATCH spice-server v5 03/18] windows: Do not use conflicting preprocessor macros

[Frediano Ziglio]

"interface" and "MAX_MONITORS" are defined in some Windows system
headers causing garbage code to be fed to the compiler.

Signed-off-by: Frediano Ziglio 
---
 server/red-qxl.c | 52 ++--
 server/reds.c| 68 
 2 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/server/red-qxl.c b/server/red-qxl.c
index 6dbd224ca..914fcfb21 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -968,95 +968,95 @@ RedsState* red_qxl_get_server(QXLState *qxl_state)
 
 void red_qxl_attach_worker(QXLInstance *qxl)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
-interface->attache_worker(qxl, >st->qxl_worker);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
+qxl_interface->attache_worker(qxl, >st->qxl_worker);
 }
 
 void red_qxl_set_compression_level(QXLInstance *qxl, int level)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
-interface->set_compression_level(qxl, level);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
+qxl_interface->set_compression_level(qxl, level);
 }
 
 void red_qxl_get_init_info(QXLInstance *qxl, QXLDevInitInfo *info)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-interface->get_init_info(qxl, info);
+qxl_interface->get_init_info(qxl, info);
 }
 
 int red_qxl_get_command(QXLInstance *qxl, struct QXLCommandExt *cmd)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-return interface->get_command(qxl, cmd);
+return qxl_interface->get_command(qxl, cmd);
 }
 
 int red_qxl_req_cmd_notification(QXLInstance *qxl)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-return interface->req_cmd_notification(qxl);
+return qxl_interface->req_cmd_notification(qxl);
 }
 
 void red_qxl_release_resource(QXLInstance *qxl, struct QXLReleaseInfoExt 
release_info)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-interface->release_resource(qxl, release_info);
+qxl_interface->release_resource(qxl, release_info);
 }
 
 int red_qxl_get_cursor_command(QXLInstance *qxl, struct QXLCommandExt *cmd)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-return interface->get_cursor_command(qxl, cmd);
+return qxl_interface->get_cursor_command(qxl, cmd);
 }
 
 int red_qxl_req_cursor_notification(QXLInstance *qxl)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-return interface->req_cursor_notification(qxl);
+return qxl_interface->req_cursor_notification(qxl);
 }
 
 void red_qxl_notify_update(QXLInstance *qxl, uint32_t update_id)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-interface->notify_update(qxl, update_id);
+qxl_interface->notify_update(qxl, update_id);
 }
 
 int red_qxl_flush_resources(QXLInstance *qxl)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-return interface->flush_resources(qxl);
+return qxl_interface->flush_resources(qxl);
 }
 
 void red_qxl_update_area_complete(QXLInstance *qxl, uint32_t surface_id,
   struct QXLRect *updated_rects,
   uint32_t num_updated_rects)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-interface->update_area_complete(qxl, surface_id, updated_rects, 
num_updated_rects);
+qxl_interface->update_area_complete(qxl, surface_id, updated_rects, 
num_updated_rects);
 }
 
 void red_qxl_set_client_capabilities(QXLInstance *qxl,
  uint8_t client_present,
  uint8_t caps[SPICE_CAPABILITIES_SIZE])
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
 if (qxl->st->running) {
-interface->set_client_capabilities(qxl, client_present, caps);
+qxl_interface->set_client_capabilities(qxl, client_present, caps);
 }
 }
 
 void red_qxl_async_complete(QXLInstance *qxl, uint64_t cookie)
 {
-QXLInterface *interface = qxl_get_interface(qxl);
+QXLInterface *qxl_interface = qxl_get_interface(qxl);
 
-interface->async_complete(qxl, cookie);
+qxl_interface->async_complete(qxl, cookie);
 }
diff --git a/server/reds.c b/server/reds.c
index 302da5858..0aae3589f 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1262,9 +1262,9 @@ void reds_release_agent_data_buffer(RedsState *reds, 
uint8_t *buf)
 static void 

[Spice-devel] [PATCH spice-server v5 13/18] windows: Disable code not working on Windows

[Frediano Ziglio]

- global signals;
- CLOEXEC flag;
- mmap and statistics;
- IPTOS_LOWDELAY flag;
- Unix sockets;
- sharing file descriptors through Unix sockets;
- TCP_CORK flag.

Signed-off-by: Frediano Ziglio 
---
 server/red-channel-client.c |  2 ++
 server/red-stream.c | 11 ++-
 server/red-stream.h |  2 ++
 server/red-worker.c |  6 ++
 server/reds.c   |  7 ++-
 server/sound.c  |  5 +++--
 server/stat-file.c  |  2 ++
 server/tests/basic-event-loop.c |  2 ++
 server/tests/replay.c   |  2 ++
 tools/Makefile.am   |  2 ++
 tools/meson.build   | 10 ++
 11 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 9aa767927..e6daddba5 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -624,6 +624,7 @@ static void 
red_channel_client_restore_main_sender(RedChannelClient *rcc)
 
 static void red_channel_client_msg_sent(RedChannelClient *rcc)
 {
+#ifndef _WIN32
 int fd;
 
 if (spice_marshaller_get_fd(rcc->priv->send_data.marshaller, )) {
@@ -637,6 +638,7 @@ static void red_channel_client_msg_sent(RedChannelClient 
*rcc)
 if (fd != -1)
 close(fd);
 }
+#endif
 
 red_channel_client_clear_sent_item(rcc);
 
diff --git a/server/red-stream.c b/server/red-stream.c
index ab3d87c7c..e8e88fb39 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -41,7 +41,7 @@
 #include "reds.h"
 
 // compatibility for *BSD systems
-#ifndef TCP_CORK
+#if !defined(TCP_CORK) && !defined(_WIN32)
 #define TCP_CORK TCP_NOPUSH
 #endif
 
@@ -102,6 +102,7 @@ struct RedStreamPrivate {
 SpiceCoreInterfaceInternal *core;
 };
 
+#ifndef _WIN32
 /**
  * Set TCP_CORK on socket
  */
@@ -111,6 +112,12 @@ static int socket_set_cork(int socket, int enabled)
 SPICE_VERIFY(sizeof(enabled) == sizeof(int));
 return setsockopt(socket, IPPROTO_TCP, TCP_CORK, , 
sizeof(enabled));
 }
+#else
+static inline int socket_set_cork(int socket, int enabled)
+{
+return -1;
+}
+#endif
 
 static ssize_t stream_write_cb(RedStream *s, const void *buf, size_t size)
 {
@@ -318,6 +325,7 @@ int red_stream_get_no_delay(RedStream *stream)
 return red_socket_get_no_delay(stream->socket);
 }
 
+#ifndef _WIN32
 int red_stream_send_msgfd(RedStream *stream, int fd)
 {
 struct msghdr msgh = { 0, };
@@ -360,6 +368,7 @@ int red_stream_send_msgfd(RedStream *stream, int fd)
 
 return r;
 }
+#endif
 
 ssize_t red_stream_writev(RedStream *s, const struct iovec *iov, int iovcnt)
 {
diff --git a/server/red-stream.h b/server/red-stream.h
index 9a7cc617e..ca6dc71a9 100644
--- a/server/red-stream.h
+++ b/server/red-stream.h
@@ -67,7 +67,9 @@ int red_stream_get_family(const RedStream *stream);
 bool red_stream_is_plain_unix(const RedStream *stream);
 bool red_stream_set_no_delay(RedStream *stream, bool no_delay);
 int red_stream_get_no_delay(RedStream *stream);
+#ifndef _WIN32
 int red_stream_send_msgfd(RedStream *stream, int fd);
+#endif
 
 /**
  * Set auto flush flag.
diff --git a/server/red-worker.c b/server/red-worker.c
index bc63fc34f..acde680ee 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1145,22 +1145,28 @@ static void *red_worker_main(void *arg)
 
 bool red_worker_run(RedWorker *worker)
 {
+#ifndef _WIN32
 sigset_t thread_sig_mask;
 sigset_t curr_sig_mask;
+#endif
 int r;
 
 spice_return_val_if_fail(worker, FALSE);
 spice_return_val_if_fail(!worker->thread, FALSE);
 
+#ifndef _WIN32
 sigfillset(_sig_mask);
 sigdelset(_sig_mask, SIGILL);
 sigdelset(_sig_mask, SIGFPE);
 sigdelset(_sig_mask, SIGSEGV);
 pthread_sigmask(SIG_SETMASK, _sig_mask, _sig_mask);
+#endif
 if ((r = pthread_create(>thread, NULL, red_worker_main, worker))) {
 spice_error("create thread failed %d", r);
 }
+#ifndef _WIN32
 pthread_sigmask(SIG_SETMASK, _sig_mask, NULL);
+#endif
 pthread_setname_np(worker->thread, "SPICE Worker");
 
 return r == 0;
diff --git a/server/reds.c b/server/reds.c
index 71a03bd0b..08b5875a4 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2646,9 +2646,11 @@ static int reds_init_socket(const char *addr, int 
portnr, int family)
 static const int on=1, off=0;
 struct addrinfo ai,*res,*e;
 char port[33];
-int slisten, rc, len;
+int slisten, rc;
 
 if (family == AF_UNIX) {
+#ifndef _WIN32
+int len;
 struct sockaddr_un local = { 0, };
 
 if ((slisten = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
@@ -2667,6 +2669,9 @@ static int reds_init_socket(const char *addr, int portnr, 
int family)
 }
 
 goto listen;
+#else
+return -1;
+#endif
 }
 
 memset(,0, sizeof(ai));
diff --git a/server/sound.c b/server/sound.c
index 167f68c57..21c1937b7 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -775,7 +775,6 @@ static void record_channel_send_item(RedChannelClient *rcc, 

[Spice-devel] [PATCH spice-server v5 12/18] test-channel: Use socket compatibility layer

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 server/tests/test-channel.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/server/tests/test-channel.c b/server/tests/test-channel.c
index fcea98aa2..372c8d79f 100644
--- a/server/tests/test-channel.c
+++ b/server/tests/test-channel.c
@@ -183,7 +183,7 @@ static void send_ack_sync(int socket, uint32_t generation)
 msg.len = GUINT32_TO_LE(sizeof(generation));
 msg.generation = GUINT32_TO_LE(generation);
 
-g_assert_cmpint(write(socket, , 10), ==, 10);
+g_assert_cmpint(socket_write(socket, , 10), ==, 10);
 }
 
 static SpiceTimer *waked_up_timer;
@@ -198,7 +198,7 @@ static void timer_wakeup(void *opaque)
 ssize_t len;
 alarm(1);
 char buffer[256];
-while ((len=recv(client_socket, buffer, sizeof(buffer), 0)) > 0)
+while ((len=socket_read(client_socket, buffer, sizeof(buffer))) > 0)
 got_data += len;
 alarm(0);
 
@@ -218,7 +218,7 @@ static void timeout_watch_count(void *opaque)
 // get all pending data
 alarm(1);
 char buffer[256];
-while (recv(client_socket, buffer, sizeof(buffer), 0) > 0)
+while (socket_read(client_socket, buffer, sizeof(buffer)) > 0)
 continue;
 alarm(0);
 
@@ -226,7 +226,7 @@ static void timeout_watch_count(void *opaque)
 watch_called_countdown = 20;
 
 // send ack reply, this should unblock data from RedChannelClient
-g_assert_cmpint(write(client_socket, "\2\0\0\0\0\0", 6), ==, 6);
+g_assert_cmpint(socket_write(client_socket, "\2\0\0\0\0\0", 6), ==, 6);
 
 // expect data soon
 waked_up_timer = core->timer_add(timer_wakeup, core);
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v5 00/18] Port SPICE server to Windows

[Frediano Ziglio]

Windows support is useful to use with Qemu under Windows as host or
to implement servers like Xspice.
Mainly SPICE server uses lot of libraries to expose a TCP protocol.
As TCP is implemented with socket library which is quite portable was
not that hard to port.
Beside some minor feature (see REAME.Windows) all was ported.
During porting was choosen to keep Unix as the main platform, if a
change would require too much changes some Windows wrapper is
preferred instead. Not too complicated stuff, the main "wrapper" is
that Windows errors from socket are written back into errno to avoid
to change lot of code handling errors.

Changes since v4:
- fix use after free in reds_set_video_codecs_from_string;
- rename conflicting identifiers instead of undefining them;
- remove some preprocessor indentation;
- improve Meson support;
- improve some comments.

Changes since v3:
- reduce compatibility layer size:
  - do not change function name if not required;
  - use int instead of a new socket_t.

Changes since v2:
- better %m replacement;
- many comments updates;
- typo and grammar fixes;
- use int to store socket descriptors/handles;
- merge all v2 updates into a single series;
- split formatting string patch.

Frediano Ziglio (18):
  build: Detect Windows build and change some definitions
  Avoids %m in formatting for Windows
  windows: Do not use conflicting preprocessor macros
  tests: Provide alarm replacement for Windows
  sys-socket: Introduce some utility to make sockets more portable
  sys-socket: Add socket_newpair utility
  net-utils: Port to Windows
  reds: Use socket compatibility layer (close -> socket_close)
  red-stream: Use socket compatibility layer
  dispatcher: Use socket compatibility layer
  test-leaks: Use socket compatibility layer
  test-channel: Use socket compatibility layer
  windows: Disable code not working on Windows
  dispatcher: Port to Windows
  tests: Exclude tests that cannot work on Windows
  red-stream: Fix SSL connection for Windows
  Disable recording filtering for Windows
  Add some notes for the Windows port

 README.Windows|  18 ++
 configure.ac  |  20 ++-
 meson.build   |  15 +-
 server/Makefile.am|   2 +
 server/dispatcher.c   |  28 ++-
 server/meson.build|   2 +
 server/net-utils.c|  11 ++
 server/red-channel-client.c   |   2 +
 server/red-common.h   |   1 +
 server/red-qxl.c  |  52 +++---
 server/red-record-qxl.c   |   7 +
 server/red-stream.c   |  48 -
 server/red-stream.h   |   2 +
 server/red-worker.c   |   6 +
 server/reds.c | 115 ++--
 server/sound.c|   5 +-
 server/stat-file.c|   2 +
 server/sys-socket.c   | 287 ++
 server/sys-socket.h   | 142 +++
 server/tests/Makefile.am  |  11 +-
 server/tests/basic-event-loop.c   |   2 +
 server/tests/meson.build  |  16 +-
 server/tests/replay.c |   2 +
 server/tests/test-channel.c   |   9 +-
 server/tests/test-leaks.c |   5 +-
 server/tests/test-loop.c  |   1 +
 server/tests/test-record.c|   7 +-
 server/tests/test-stream-device.c |   1 +
 server/tests/win-alarm.c  |  65 +++
 server/tests/win-alarm.h  |  26 +++
 tools/Makefile.am |   2 +
 tools/meson.build |  10 +-
 32 files changed, 805 insertions(+), 117 deletions(-)
 create mode 100644 README.Windows
 create mode 100644 server/sys-socket.c
 create mode 100644 server/sys-socket.h
 create mode 100644 server/tests/win-alarm.c
 create mode 100644 server/tests/win-alarm.h

-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel