[Spice-commits] 5 commits - .gitlab-ci.yml server/glib-compat.h server/Makefile.am server/meson.build server/red-channel-capabilities.c server/red-parse-qxl.cpp server/reds.cpp server/red-stream-device.cpp server/smartcard.cpp server/sound.cpp server/tests server/websocket.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed May 5 05:37:03 UTC 2021


 .gitlab-ci.yml                    |    8 ++--
 server/Makefile.am                |    1 
 server/glib-compat.h              |   49 ++++++++++++++++++++++++++++
 server/meson.build                |    1 
 server/red-channel-capabilities.c |    5 +-
 server/red-parse-qxl.cpp          |    4 +-
 server/red-stream-device.cpp      |    3 +
 server/reds.cpp                   |    7 ++--
 server/smartcard.cpp              |    3 +
 server/sound.cpp                  |    3 +
 server/tests/test-channel.cpp     |    2 -
 server/tests/test-smartcard.cpp   |    5 +-
 server/tests/valgrind/glib.supp   |   66 ++++++++++++++++++++++++++++++++++++--
 server/tests/valgrind/spice.supp  |   11 ++++++
 server/websocket.c                |    3 +
 15 files changed, 151 insertions(+), 20 deletions(-)

New commits:
commit 127c047ad72f35c16a4238fb4680d46455da31d5
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 4 17:49:52 2021 +0100

    ci: Enable -Werror again
    
    By default after commit b24da370749d (cfr: "build: Disable
    default -Werror if source is a git repository") -Werror is
    by default disabled using Autoconf.
    Enabled for the CI.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 7d3ba538..a0b30e18 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -33,7 +33,7 @@ makecheck:
     CFLAGS='-O2 -pipe -g -fsanitize=address -fno-omit-frame-pointer -Wframe-larger-than=40920'
     CXXFLAGS='-O2 -pipe -g -fsanitize=address -fno-omit-frame-pointer -Wframe-larger-than=40920'
     LDFLAGS='-fsanitize=address -lasan'
-    ../autogen.sh
+    ../autogen.sh --enable-werror
   - make
   - make -C server check || (cat server/tests/test-suite.log && exit 1)
 
@@ -54,7 +54,7 @@ meson-makecheck:
 # --without-sasl       disable SASL
 options:
   script:
-  - ./autogen.sh --enable-statistics --without-sasl
+  - ./autogen.sh --enable-statistics --without-sasl --enable-werror
   - make
   - make -C server check || (cat server/tests/test-suite.log && exit 1)
 
@@ -88,7 +88,7 @@ syntax-check:
 
 distcheck:
   script:
-  - ./autogen.sh --enable-manual
+  - ./autogen.sh --enable-manual --enable-werror
   - make distcheck
 
 # Same as makecheck job but use a Centos image
@@ -112,7 +112,7 @@ makecheck-centos:
     CFLAGS='-O2 -pipe -g -fsanitize=address -fno-omit-frame-pointer -Wframe-larger-than=40920'
     CXXFLAGS='-O2 -pipe -g -fsanitize=address -fno-omit-frame-pointer -Wframe-larger-than=40920'
     LDFLAGS='-fsanitize=address -lasan'
-    ./autogen.sh
+    ./autogen.sh --enable-werror
   - make
   - make -C server check || (cat server/tests/test-suite.log && exit 1)
 
commit 62dd7e47e9eed118d012758c4a7297ffa9978868
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 4 18:11:40 2021 +0100

    tests: Remove some compiler warnings
    
    Remove warnings like:
    
    In file included from /usr/include/glib-2.0/glib.h:86,
                     from ../server/tests/test-glib-compat.h:21,
                     from ../server/tests/test-channel.cpp:25:
    In function 'void send_ack_sync(int, uint32_t)',
        inlined from 'void channel_loop()' at ../server/tests/test-channel.cpp:250:18:
    ../server/sys-socket.h:28:43: error: 'ssize_t write(int, const void*, size_t)' reading 10 bytes from a region of size 2 [-Werror=stringop-overread]
       28 | #define socket_write(sock, buf, len) write(sock, buf, len)
    /usr/include/glib-2.0/glib/gtestutils.h:50:61: note: in definition of macro 'g_assert_cmpint'
       50 |                                              gint64 __n1 = (n1), __n2 = (n2); \
          |                                                             ^~
    ../server/tests/test-channel.cpp:132:21: note: in expansion of macro 'socket_write'
      132 |     g_assert_cmpint(socket_write(socket, &msg.type, 10), ==, 10);
          |                     ^~~~~~~~~~~~
    ../server/tests/test-channel.cpp: In function 'void channel_loop()':
    ../server/tests/test-channel.cpp:123:18: note: source object 'send_ack_sync(int, uint32_t)::<unnamed struct>::type' of size 2
      123 |         uint16_t type;
          |                  ^~~~
    In file included from ../server/tests/test-channel.cpp:22:
    /usr/include/unistd.h:367:16: note: in a call to function 'ssize_t write(int, const void*, size_t)' declared with attribute 'access (read_only, 2, 3)'
      367 | extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
          |                ^~~~~
    cc1plus: all warnings being treated as errors
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/server/tests/test-channel.cpp b/server/tests/test-channel.cpp
index 036f4992..ffd0ec0f 100644
--- a/server/tests/test-channel.cpp
+++ b/server/tests/test-channel.cpp
@@ -129,7 +129,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(socket_write(socket, &msg.type, 10), ==, 10);
+    g_assert_cmpint(socket_write(socket, reinterpret_cast<uint8_t *>(&msg) + 2, 10), ==, 10);
 }
 
 static SpiceTimer *waked_up_timer;
diff --git a/server/tests/test-smartcard.cpp b/server/tests/test-smartcard.cpp
index 8c127e2e..f76f602e 100644
--- a/server/tests/test-smartcard.cpp
+++ b/server/tests/test-smartcard.cpp
@@ -108,7 +108,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(socket_write(socket, &msg.type, 10), ==, 10);
+    g_assert_cmpint(socket_write(socket, reinterpret_cast<uint8_t *>(&msg) + 2, 10), ==, 10);
 }
 
 static void send_data(int socket, uint32_t type, uint32_t reader_id)
@@ -128,7 +128,8 @@ static void send_data(int socket, uint32_t type, uint32_t reader_id)
     msg.vheader.length = GUINT32_TO_LE(6);
     strcpy(msg.data, "hello");
 
-    g_assert_cmpint(socket_write(socket, &msg.type, sizeof(msg)-4), ==, sizeof(msg)-4);
+    g_assert_cmpint(socket_write(socket, reinterpret_cast<uint8_t *>(&msg) + 2,
+                                 sizeof(msg) - 4), ==, sizeof(msg) - 4);
 }
 
 static void check_data(VmcEmu *vmc_emu)
commit e39412644c468e26c4bfc59e8f077d4de4200ecd
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 4 19:05:00 2021 +0100

    ci: Ignore a leak in glib threading pool
    
    The leak is detected by Valgrind on Fedora 34as:
    
    ==19603== 400 bytes in 1 blocks are possibly lost in loss record 2,296 of 2,441
    ==19603==    at 0x4845464: calloc (vg_replace_malloc.c:1117)
    ==19603==    by 0x40135FB: _dl_allocate_tls (in /usr/lib64/ld-2.33.so)
    ==19603==    by 0x57EB008: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.33.so)
    ==19603==    by 0x53A1130: UnknownInlinedFun (gthread-posix.c:1323)
    ==19603==    by 0x53A1130: g_thread_new_internal (gthread.c:931)
    ==19603==    by 0x53C4953: g_thread_pool_start_thread.constprop.0 (gthreadpool.c:477)
    ==19603==    by 0x53A2902: g_thread_pool_push (gthreadpool.c:691)
    ==19603==    by 0x519AE11: g_task_run_in_thread_sync (gtask.c:1593)
    ==19603==    by 0x80D8A74: ??? (in /usr/lib64/gio/modules/libgiolibproxy.so)
    ==19603==    by 0x5181966: g_proxy_address_enumerator_next (gproxyaddressenumerator.c:176)
    ==19603==    by 0x519281A: g_socket_client_connect (gsocketclient.c:1098)
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/server/tests/valgrind/spice.supp b/server/tests/valgrind/spice.supp
index 63203dcc..97d8d80d 100644
--- a/server/tests/valgrind/spice.supp
+++ b/server/tests/valgrind/spice.supp
@@ -124,3 +124,14 @@
     ...
     fun:g_thread_new
 }
+
+{
+    g_thread_pool_leak
+    Memcheck:Leak
+    match-leak-kinds:possible
+    fun:calloc
+    ...
+    fun:g_thread_new_internal
+    ...
+    fun:g_thread_pool_push
+}
commit 7fe49b6465bfda7f7f9c84e749a5856e5a54bde4
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 4 17:46:40 2021 +0100

    Update glib.supp to new version
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp
index 06c5f542..28316636 100644
--- a/server/tests/valgrind/glib.supp
+++ b/server/tests/valgrind/glib.supp
@@ -90,7 +90,7 @@
 	match-leak-kinds:reachable
 	fun:malloc
 	...
-	fun:gobject_init_ctor
+	fun:gobject_init*
 }
 
 {
@@ -99,7 +99,7 @@
 	match-leak-kinds:reachable
 	fun:realloc
 	...
-	fun:gobject_init_ctor
+	fun:gobject_init*
 }
 
 {
@@ -108,7 +108,7 @@
 	match-leak-kinds:possible,reachable
 	fun:calloc
 	...
-	fun:gobject_init_ctor
+	fun:gobject_init*
 }
 
 {
@@ -366,6 +366,16 @@
         fun:g_type_class_ref
 }
 
+{
+	g-type-class-ref-inlined
+	Memcheck:Leak
+	fun:calloc
+	...
+	fun:UnknownInlinedFun
+	...
+	fun:g_type_class_ref
+}
+
 {
 	g-io-module-default-singleton-malloc
 	Memcheck:Leak
@@ -421,6 +431,14 @@
 	fun:_g_io_module_get_default*
 }
 
+{
+	g-io-module-default-singleton-weak-ref
+	Memcheck:Leak
+	fun:calloc
+	...
+	fun:_g_io_module_get_default
+}
+
 {
 	g-get-language-names-malloc
 	Memcheck:Leak
@@ -484,6 +502,15 @@
 	fun:g_system_thread_new
 }
 
+{
+	g-system-thread-init-malloc
+	Memcheck:Leak
+	match-leak-kinds:possible,reachable
+	fun:malloc
+	...
+	fun:g_system_thread_new
+}
+
 {
 	g-task-thread-pool-init
 	Memcheck:Leak
@@ -1038,3 +1065,36 @@
 	...
 	fun:g_set_prgname
 }
+
+# Error domains hash
+{
+	g_error_init
+	Memcheck:Leak
+	match-leak-kinds: reachable
+	fun:malloc
+	...
+	fun:g_hash_table_new_full
+	fun:g_error_init
+}
+
+# Error domain static registration
+{
+	g_error_domain_register_static
+	Memcheck:Leak
+	match-leak-kinds: reachable
+	fun:malloc
+	...
+	fun:g_hash_table_insert
+	fun:error_domain_register
+	fun:g_error_domain_register_static
+}
+
+{
+	new_quark
+	Memcheck:Leak
+	match-leak-kinds:reachable
+	fun:malloc
+	...
+	fun:g_hash_table_insert
+	fun:quark_new
+}
commit 8c458fa35e8979bb088cbc6d72f7bccd73f99189
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 4 13:04:27 2021 +0100

    Fix g_memdup deprecation warning with glib >= 2.68
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/server/Makefile.am b/server/Makefile.am
index 2b364cad..73e7cdf9 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -107,6 +107,7 @@ libserver_la_SOURCES =				\
 	display-channel-private.h		\
 	display-limits.h			\
 	event-loop.c				\
+	glib-compat.h				\
 	glz-encoder.c				\
 	glz-encoder-dict.c			\
 	glz-encoder-dict.h			\
diff --git a/server/glib-compat.h b/server/glib-compat.h
new file mode 100644
index 00000000..5f36d3e2
--- /dev/null
+++ b/server/glib-compat.h
@@ -0,0 +1,49 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2021 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 <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef GLIB_COMPAT_H_
+#define GLIB_COMPAT_H_
+
+#include <glib.h>
+
+#if GLIB_VERSION_MIN_REQUIRED >= G_ENCODE_VERSION(2, 68)
+#error Time to remove this section
+#elif !GLIB_CHECK_VERSION(2,68,0)
+static inline void*
+g_memdup2(const void *ptr, size_t size)
+{
+    void *dst = NULL;
+
+    if (ptr && size != 0) {
+        dst = g_malloc(size);
+        memcpy(dst, ptr, size);
+    }
+    return dst;
+}
+#elif GLIB_VERSION_MAX_ALLOWED < G_ENCODE_VERSION(2, 68)
+static inline void*
+g_memdup2_compat(const void *ptr, size_t size)
+{
+    G_GNUC_BEGIN_IGNORE_DEPRECATIONS
+    return g_memdup2(ptr, size);
+    G_GNUC_END_IGNORE_DEPRECATIONS
+}
+#define g_memdup2 g_memdup2_compat
+#endif
+
+#endif /* GLIB_COMPAT_H_ */
diff --git a/server/meson.build b/server/meson.build
index 4a670635..f9cdf9de 100644
--- a/server/meson.build
+++ b/server/meson.build
@@ -78,6 +78,7 @@ spice_server_sources = [
   'display-channel-private.h',
   'display-limits.h',
   'event-loop.c',
+  'glib-compat.h',
   'glz-encoder.c',
   'glz-encoder-dict.c',
   'glz-encoder-dict.h',
diff --git a/server/red-channel-capabilities.c b/server/red-channel-capabilities.c
index 61685aac..f4b7fad1 100644
--- a/server/red-channel-capabilities.c
+++ b/server/red-channel-capabilities.c
@@ -20,6 +20,7 @@
 #include <glib.h>
 #include <common/macros.h>
 
+#include "glib-compat.h"
 #include "red-channel-capabilities.h"
 
 void red_channel_capabilities_init(RedChannelCapabilities *dest,
@@ -27,11 +28,11 @@ void red_channel_capabilities_init(RedChannelCapabilities *dest,
 {
     *dest = *caps;
     if (caps->common_caps) {
-        dest->common_caps = (uint32_t*) g_memdup(caps->common_caps,
+        dest->common_caps = (uint32_t*) g_memdup2(caps->common_caps,
                                      caps->num_common_caps * sizeof(uint32_t));
     }
     if (caps->num_caps) {
-        dest->caps = (uint32_t*) g_memdup(caps->caps, caps->num_caps * sizeof(uint32_t));
+        dest->caps = (uint32_t*) g_memdup2(caps->caps, caps->num_caps * sizeof(uint32_t));
     }
 }
 
diff --git a/server/red-parse-qxl.cpp b/server/red-parse-qxl.cpp
index 35754362..979d1b8f 100644
--- a/server/red-parse-qxl.cpp
+++ b/server/red-parse-qxl.cpp
@@ -20,6 +20,8 @@
 #include <inttypes.h>
 #include <glib.h>
 #include <common/lz_common.h>
+
+#include "glib-compat.h"
 #include "spice-bitmap-utils.h"
 #include "red-common.h"
 #include "red-qxl.h"
@@ -1562,7 +1564,7 @@ static bool red_get_cursor(RedMemSlotInfo *slots, int group_id,
     if (free_data) {
         red->data = data;
     } else {
-        red->data = (uint8_t*) g_memdup(data, size);
+        red->data = (uint8_t*) g_memdup2(data, size);
     }
     // Arrived here we could note that we are not going to use anymore cursor data
     // and we could be tempted to release resource back to QXL. Don't do that!
diff --git a/server/red-stream-device.cpp b/server/red-stream-device.cpp
index 224187dc..fe53a3c8 100644
--- a/server/red-stream-device.cpp
+++ b/server/red-stream-device.cpp
@@ -19,6 +19,7 @@
 
 #include <common/recorder.h>
 
+#include "glib-compat.h"
 #include "red-stream-device.h"
 
 #include "stream-channel.h"
@@ -416,7 +417,7 @@ stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg, size_t msg_si
         return nullptr;
     }
     cursor->data_size = size_required;
-    cursor->data = (uint8_t*) g_memdup(msg->data, size_required);
+    cursor->data = (uint8_t*) g_memdup2(msg->data, size_required);
     return cmd;
 }
 
diff --git a/server/reds.cpp b/server/reds.cpp
index fa6e393a..b31a6a5c 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -53,6 +53,7 @@
 #include <common/generated_server_marshallers.h>
 #include <common/agent.h>
 
+#include "glib-compat.h"
 #include "spice-wrapped.h"
 #include "reds.h"
 #include "agent-msg-filter.h"
@@ -1458,7 +1459,7 @@ bool reds_handle_migrate_data(RedsState *reds, MainChannelClient *mcc,
             /* restore agent state when the agent gets attached */
             spice_debug("saving mig_data");
             spice_assert(agent_dev->priv->plug_generation == 0);
-            agent_dev->priv->mig_data = (SpiceMigrateDataMain*) g_memdup(mig_data, size);
+            agent_dev->priv->mig_data = (SpiceMigrateDataMain*) g_memdup2(mig_data, size);
         }
     } else {
         spice_debug("agent was not attached on the source host");
@@ -1746,13 +1747,13 @@ red_channel_capabilities_init_from_link_message(RedChannelCapabilities *caps,
     caps->num_common_caps = link_mess->num_common_caps;
     caps->common_caps = nullptr;
     if (caps->num_common_caps) {
-        caps->common_caps = (uint32_t*) g_memdup(raw_caps,
+        caps->common_caps = (uint32_t*) g_memdup2(raw_caps,
                                      link_mess->num_common_caps * sizeof(uint32_t));
     }
     caps->num_caps = link_mess->num_channel_caps;
     caps->caps = nullptr;
     if (link_mess->num_channel_caps) {
-        caps->caps = (uint32_t*) g_memdup(raw_caps + link_mess->num_common_caps * sizeof(uint32_t),
+        caps->caps = (uint32_t*) g_memdup2(raw_caps + link_mess->num_common_caps * sizeof(uint32_t),
                               link_mess->num_channel_caps * sizeof(uint32_t));
     }
 }
diff --git a/server/smartcard.cpp b/server/smartcard.cpp
index e76ec033..709815a0 100644
--- a/server/smartcard.cpp
+++ b/server/smartcard.cpp
@@ -22,6 +22,7 @@
 #include <libcacard.h>
 #endif
 
+#include "glib-compat.h"
 #include "reds.h"
 #include "char-device.h"
 #include "smartcard.h"
@@ -389,7 +390,7 @@ smartcard_new_vsc_msg_item(unsigned int reader_id, const VSCMsgHeader *vheader)
 {
     auto msg_item = red::make_shared<RedMsgItem>();
 
-    msg_item->vheader.reset((VSCMsgHeader*) g_memdup(vheader, sizeof(*vheader) + vheader->length));
+    msg_item->vheader.reset((VSCMsgHeader*) g_memdup2(vheader, sizeof(*vheader) + vheader->length));
     /* We patch the reader_id, since the device only knows about itself, and
      * we know about the sum of readers. */
     msg_item->vheader->reader_id = reader_id;
diff --git a/server/sound.cpp b/server/sound.cpp
index 66743af5..0c930191 100644
--- a/server/sound.cpp
+++ b/server/sound.cpp
@@ -31,6 +31,7 @@
 #include <common/generated_server_marshallers.h>
 #include <common/snd_codec.h>
 
+#include "glib-compat.h"
 #include "spice-wrapped.h"
 #include "red-common.h"
 #include "main-channel.h"
@@ -790,7 +791,7 @@ static void snd_channel_set_volume(SndChannel *channel,
 
     st->volume_nchannels = nchannels;
     g_free(st->volume);
-    st->volume = (uint16_t*) g_memdup(volume, sizeof(uint16_t) * nchannels);
+    st->volume = (uint16_t*) g_memdup2(volume, sizeof(uint16_t) * nchannels);
 
     if (!client || nchannels == 0)
         return;
diff --git a/server/websocket.c b/server/websocket.c
index 6b974cce..f9b9ea32 100644
--- a/server/websocket.c
+++ b/server/websocket.c
@@ -36,6 +36,7 @@
 #include <common/mem.h>
 
 #include "sys-socket.h"
+#include "glib-compat.h"
 #include "websocket.h"
 
 #ifdef _WIN32
@@ -487,7 +488,7 @@ static void constrain_iov(struct iovec *iov, int iovcnt,
         if (iov[i].iov_len > maxlen) {
             /* TODO - This code has never triggered afaik... */
             *iov_out_cnt = ++i;
-            *iov_out = g_memdup(iov, i * sizeof (*iov));
+            *iov_out = g_memdup2(iov, i * sizeof (*iov));
             (*iov_out)[i-1].iov_len = maxlen;
             return;
         }


More information about the Spice-commits mailing list