[Spice-commits] 4 commits - configure.ac spice-common src/channel-cursor.c src/channel-display-mjpeg.c src/channel-main.c src/continuation.h src/decode-glz-tmpl.c src/spice-channel.c src/spice-widget.c

Christophe Fergau teuf at kemper.freedesktop.org
Tue Jun 20 13:21:48 UTC 2017


 configure.ac                |   23 +++++++++++++++++++++++
 spice-common                |    2 +-
 src/channel-cursor.c        |    8 +++++---
 src/channel-display-mjpeg.c |    2 +-
 src/channel-main.c          |    2 +-
 src/continuation.h          |    6 ++++--
 src/decode-glz-tmpl.c       |    2 +-
 src/spice-channel.c         |   25 +++++++++++++++----------
 src/spice-widget.c          |    7 ++++---
 9 files changed, 55 insertions(+), 22 deletions(-)

New commits:
commit 34f07605531124a6176b83170d704f531e38ff84
Author: Christophe de Dinechin <dinechin at redhat.com>
Date:   Thu Jun 8 17:38:54 2017 +0200

    Remove warning about unused variable when building on macOS
    
    On macOS, neither of the two cases implemented in set_mouse_accel applies.
    We get the following eror message:
    
      CC       spice-widget.lo
    spice-widget.c:944:26: error: unused variable 'd' [-Werror,-Wunused-variable]
        SpiceDisplayPrivate *d = display->priv;
    
    Acked-by: Frediano Ziglio <fziglio at redhat.com>
    Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/src/spice-widget.c b/src/spice-widget.c
index 5be6028..6f4abc0 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -949,9 +949,8 @@ static void update_keyboard_grab(SpiceDisplay *display)
 
 static void set_mouse_accel(SpiceDisplay *display, gboolean enabled)
 {
-    SpiceDisplayPrivate *d = display->priv;
-
 #if defined GDK_WINDOWING_X11
+    SpiceDisplayPrivate *d = display->priv;
     GdkWindow *w = GDK_WINDOW(gtk_widget_get_window(GTK_WIDGET(display)));
 
     if (!GDK_IS_X11_DISPLAY(gdk_window_get_display(w))) {
@@ -973,6 +972,7 @@ static void set_mouse_accel(SpiceDisplay *display, gboolean enabled)
                       d->x11_accel_numerator, d->x11_accel_denominator, d->x11_threshold);
     }
 #elif defined GDK_WINDOWING_WIN32
+    SpiceDisplayPrivate *d = display->priv;
     if (enabled) {
         g_return_if_fail(SystemParametersInfo(SPI_SETMOUSE, 0, &d->win_mouse, 0));
         g_return_if_fail(SystemParametersInfo(SPI_SETMOUSESPEED, 0, (PVOID)(INT_PTR)d->win_mouse_speed, 0));
@@ -984,6 +984,7 @@ static void set_mouse_accel(SpiceDisplay *display, gboolean enabled)
         g_return_if_fail(SystemParametersInfo(SPI_SETMOUSESPEED, 0, (PVOID)10, SPIF_SENDCHANGE)); // default
     }
 #else
+    /* TODO: Add mouse acceleration for macOS */
     g_warning("Mouse acceleration code missing for your platform");
 #endif
 }
commit babe5630d5d3242b1d186cccdd5b4d51debe78e9
Author: Christophe de Dinechin <dinechin at redhat.com>
Date:   Thu Jun 8 17:38:53 2017 +0200

    Avoid clang warnings on casts with stricter alignment requirements
    
    For example, something like this:
        uint8_t  *p8;
        uint32_t *p32 = (uint32_t *) p8;
    
    generates a warning like this:
      spice-channel.c:1350:10: error: cast from 'uint8_t *' (aka 'unsigned char *') to
          'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to
          4 [-Werror,-Wcast-align]
    
    The warning indicates that we end up with a pointer to data that
    should be 4-byte aligned, but its value may be misaligned. On x86,
    this does not make much of a difference, except a relatively minor
    performance penalty. However, on platforms such as older ARM, misaligned
    accesses are emulated by the kernel, and support for them is optional.
    So we may end up with a fault.
    
    The intent of the fix here is to make it easy to identify and rework
    places where actual mis-alignment occurs. Wherever casts raise the warning,
    they are replaced with a macro:
    
    - SPICE_ALIGNED_CAST(type, value) casts value to type, and indicates that
      we believe the resulting pointer is aligned. If it is not, a runtime
      warning will be issued if you configure with --enable-alignment-warnings.
    
    - SPICE_UNALIGNED_CAST(type, value) casts value to type, and indicates that
      we believe the resulting pointer is not always aligned. A debug message
      is issued if you configure with --enable-alignment-warnings.
    
    Any code using SPICE_UNALIGNED_CAST may need to be revisited in order
    to improve performance, e.g. by using memcpy. A few easy fixes have been
    made in his patch.
    
    Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/configure.ac b/configure.ac
index 62acafc..caa289a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -565,6 +565,14 @@ else
   SPICE_WARNING([No D-Bus support, desktop integration and USB redirection may not work properly])
 fi
 
+AC_ARG_ENABLE([alignment-checks],
+  AS_HELP_STRING([--enable-alignment-checks],
+                 [Enable runtime checks for cast alignment @<:@default=no@:>@]),
+  [],
+  enable_alignment_checks="no")
+AS_IF([test "x$enable_alignment_checks" = "xyes"],
+      [AC_DEFINE([SPICE_DEBUG_ALIGNMENT], 1, [Enable runtime checks for cast alignment])])
+
 SPICE_CHECK_LZ4
 
 dnl ===========================================================================
diff --git a/spice-common b/spice-common
index af682b1..858a0bf 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit af682b1b06dea55007d9aa7c37cd443e4349e43f
+Subproject commit 858a0bfae9925ea6e363573de4113090c7821133
diff --git a/src/channel-cursor.c b/src/channel-cursor.c
index 558c920..14053a9 100644
--- a/src/channel-cursor.c
+++ b/src/channel-cursor.c
@@ -381,10 +381,11 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
     SpiceCursorHeader *hdr = &scursor->header;
     display_cursor *cursor;
     size_t size;
-    gint i, pix_mask, pix;
+    guint32 i, pix_mask, pix;
     const guint8* data;
     guint8 *rgba;
     guint8 val;
+    guint32 palette[16];
 
     CHANNEL_DEBUG(channel, "%s: flags %x, size %u", __FUNCTION__,
                   scursor->flags, scursor->data_size);
@@ -433,7 +434,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
         size /= 2u;
         for (i = 0; i < hdr->width * hdr->height; i++) {
             pix_mask = get_pix_mask(data, size, i);
-            pix = *((guint16*)data + i);
+            pix = *(SPICE_UNALIGNED_CAST(guint16 *, data) + i);
             if (pix_mask && pix == 0x7fff) {
                 cursor->data[i] = get_pix_hack(i, hdr->width);
             } else {
@@ -444,10 +445,11 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
         break;
     case SPICE_CURSOR_TYPE_COLOR4:
         size = ((unsigned int)(SPICE_ALIGN(hdr->width, 2) / 2)) * hdr->height;
+        memcpy(palette, data + size, sizeof(palette));
         for (i = 0; i < hdr->width * hdr->height; i++) {
             pix_mask = get_pix_mask(data, size + (sizeof(uint32_t) << 4), i);
             int idx = (i & 1) ? (data[i >> 1] & 0x0f) : ((data[i >> 1] & 0xf0) >> 4);
-            pix = *((uint32_t*)(data + size) + idx);
+            pix = palette[idx];
             if (pix_mask && pix == 0xffffff) {
                 cursor->data[i] = get_pix_hack(i, hdr->width);
             } else {
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 3ae9d21..ee33b01 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -151,7 +151,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder)
 #ifndef JCS_EXTENSIONS
         {
             uint8_t *s = lines[0];
-            uint32_t *d = (uint32_t *)s;
+            uint32_t *d = SPICE_ALIGNED_CAST(uint32_t *, s);
 
             if (back_compat) {
                 for (unsigned int j = lines_read * width; j > 0; ) {
diff --git a/src/channel-main.c b/src/channel-main.c
index 29dd42d..1202d13 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1872,7 +1872,7 @@ static void main_agent_handle_xfer_status(SpiceMainChannel *channel,
                                     _("The spice agent reported an error during the file transfer"));
         break;
     case VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE: {
-        uint64_t *free_space = (uint64_t *)(msg->data);
+        uint64_t *free_space = SPICE_ALIGNED_CAST(uint64_t *, msg->data);
         gchar *free_space_str = g_format_size(*free_space);
         gchar *file_size_str = g_format_size(spice_file_transfer_task_get_total_bytes(xfer_task));
         error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
diff --git a/src/continuation.h b/src/continuation.h
index 675a257..d1fd137 100644
--- a/src/continuation.h
+++ b/src/continuation.h
@@ -21,6 +21,7 @@
 #ifndef _CONTINUATION_H_
 #define _CONTINUATION_H_
 
+#include "spice-common.h"
 #include <stddef.h>
 #include <ucontext.h>
 #include <setjmp.h>
@@ -48,8 +49,9 @@ int cc_release(struct continuation *cc);
 int cc_swap(struct continuation *from, struct continuation *to);
 
 #define offset_of(type, member) ((unsigned long)(&((type *)0)->member))
-#define container_of(obj, type, member) \
-        (type *)(((char *)obj) - offset_of(type, member))
+#define container_of(obj, type, member)                                 \
+        SPICE_ALIGNED_CAST(type *,                                      \
+                           (((char *)obj) - offset_of(type, member)))
 
 #endif
 /*
diff --git a/src/decode-glz-tmpl.c b/src/decode-glz-tmpl.c
index b337a8b..76d832c 100644
--- a/src/decode-glz-tmpl.c
+++ b/src/decode-glz-tmpl.c
@@ -178,7 +178,7 @@ static size_t FNAME(decode)(SpiceGlzDecoderWindow *window,
                             uint64_t image_id, SpicePalette *plt)
 {
     uint8_t      *ip = in_buf;
-    OUT_PIXEL    *out_pix_buf = (OUT_PIXEL *)out_buf;
+    OUT_PIXEL    *out_pix_buf = SPICE_ALIGNED_CAST(OUT_PIXEL *, out_buf);
     OUT_PIXEL    *op = out_pix_buf;
     OUT_PIXEL    *op_limit = out_pix_buf + size;
 
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 77ac9cd..418e2b7 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1312,6 +1312,7 @@ static void spice_channel_send_link(SpiceChannel *channel)
 {
     SpiceChannelPrivate *c = channel->priv;
     uint8_t *buffer, *p;
+    uint32_t *caps;
     int protocol, i;
     SpiceLinkMess link_msg;
 
@@ -1357,14 +1358,14 @@ static void spice_channel_send_link(SpiceChannel *channel)
     memcpy(p, &c->link_hdr, sizeof(c->link_hdr)); p += sizeof(c->link_hdr);
     memcpy(p, &link_msg, sizeof(link_msg)); p += sizeof(link_msg);
 
+    caps = SPICE_UNALIGNED_CAST(uint32_t *,p);
     for (i = 0; i < c->common_caps->len; i++) {
-        *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, i));
-        p += sizeof(uint32_t);
+        *caps++ = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, i));
     }
     for (i = 0; i < c->caps->len; i++) {
-        *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i));
-        p += sizeof(uint32_t);
+        *caps++ = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i));
     }
+    p = (uint8_t *) caps;
     CHANNEL_DEBUG(channel, "channel type %d id %d num common caps %u num caps %u",
                   c->channel_type,
                   c->channel_id,
@@ -1888,6 +1889,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
     SpiceChannelPrivate *c;
     int rc, num_caps, i;
     uint32_t *caps, num_channel_caps, num_common_caps;
+    uint8_t *caps_src;
     SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;
 
     g_return_val_if_fail(channel != NULL, FALSE);
@@ -1927,18 +1929,21 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
     /* see original spice/client code: */
     /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */
 
-    caps = (uint32_t *)((uint8_t *)c->peer_msg + GUINT32_FROM_LE(c->peer_msg->caps_offset));
-
+    caps_src = (uint8_t *)c->peer_msg + GUINT32_FROM_LE(c->peer_msg->caps_offset);
     g_array_set_size(c->remote_common_caps, num_common_caps);
+    caps = &g_array_index(c->remote_common_caps, uint32_t, 0);
+    memcpy(caps, caps_src, num_common_caps * sizeof(uint32_t));
     for (i = 0; i < num_common_caps; i++, caps++) {
-        g_array_index(c->remote_common_caps, uint32_t, i) = GUINT32_FROM_LE(*caps);
-        CHANNEL_DEBUG(channel, "got common caps %d:0x%X", i, GUINT32_FROM_LE(*caps));
+        *caps = GUINT32_FROM_LE(*caps);
+        CHANNEL_DEBUG(channel, "got common caps %d:0x%X", i, *caps);
     }
 
     g_array_set_size(c->remote_caps, num_channel_caps);
+    caps_src += num_common_caps * sizeof(uint32_t);
+    memcpy(caps, caps_src, num_channel_caps * sizeof(uint32_t));
     for (i = 0; i < num_channel_caps; i++, caps++) {
-        g_array_index(c->remote_caps, uint32_t, i) = GUINT32_FROM_LE(*caps);
-        CHANNEL_DEBUG(channel, "got channel caps %d:0x%X", i, GUINT32_FROM_LE(*caps));
+        *caps = GUINT32_FROM_LE(*caps);
+        CHANNEL_DEBUG(channel, "got channel caps %d:0x%X", i, *caps);
     }
 
     if (!spice_channel_test_common_capability(channel,
commit 5db5f1f3b5761e532e664dd6a441051e9e72d264
Author: Christophe de Dinechin <dinechin at redhat.com>
Date:   Thu Jun 8 17:38:52 2017 +0200

    Add check for macOS and macOS specific define to allow ucontext
    
    Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/configure.ac b/configure.ac
index ff00d73..62acafc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -62,6 +62,18 @@ esac
 AC_MSG_RESULT([$os_win32])
 AM_CONDITIONAL([OS_WIN32],[test "$os_win32" = "yes"])
 
+AC_MSG_CHECKING([for native macOS])
+case "$host_os" in
+     *darwin*)
+        os_mac=yes
+        ;;
+     *)
+        os_mac=no
+        ;;
+esac
+AC_MSG_RESULT([$os_mac])
+AM_CONDITIONAL([OS_MAC],[test "$os_mac" = "yes"])
+
 AC_CHECK_HEADERS([sys/socket.h netinet/in.h arpa/inet.h])
 AC_CHECK_HEADERS([termios.h])
 AC_CHECK_HEADERS([epoxy/egl.h],
@@ -468,6 +480,9 @@ esac
 if test "$with_coroutine" = "auto"; then
   if test "$os_win32" = "yes"; then
     with_coroutine=winfiber
+  elif test "$os_mac" = "yes"; then
+    with_coroutine=ucontext
+    AC_DEFINE([_XOPEN_SOURCE], [1], [Define _XOPEN_SOURCE on macOS for ucontext])
   else
     with_coroutine=ucontext
   fi
commit 0be8a492c1a36956cf922a4522e067503e80fd55
Author: Christophe de Dinechin <dinechin at redhat.com>
Date:   Thu Jun 8 17:38:51 2017 +0200

    Remove trailing space
    
    Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/src/spice-widget.c b/src/spice-widget.c
index c335083..5be6028 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1630,7 +1630,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
 
     if (key->keyval == GDK_KEY_Pause
 #ifdef G_OS_WIN32
-        /* for some reason GDK does not fill keyval for VK_PAUSE 
+        /* for some reason GDK does not fill keyval for VK_PAUSE
          * See https://bugzilla.gnome.org/show_bug.cgi?id=769214
          */
         || key->hardware_keycode == VK_PAUSE


More information about the Spice-commits mailing list