From 7a9af7213bd95d180bb0e8e7b0efa96a5a762f28 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 12:55:06 +0100 Subject: [PATCH 01/10] shared/tests: add nmtst_g_source_nop() helper (cherry picked from commit ca2b79d9aa9999bdb09b31599a4bd942122b282e) (cherry picked from commit ddb69b211ce2aaeda45ca4afb664b25e9c0dfd94) (cherry picked from commit e069982492440203b9b7b4982d8904146c85c0b9) (cherry picked from commit 8358652425b971ed231d56f79c275a8bb6ccab41) (cherry picked from commit 2d59d02d3354bf6cacbad6627940d664718fdc89) --- shared/nm-utils/nm-test-utils.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index d0c1937021..c3d6447f80 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -999,6 +999,13 @@ nmtst_g_source_set_boolean_true (gpointer user_data) return G_SOURCE_CONTINUE; } +static inline gboolean +nmtst_g_source_nop (gpointer user_data) +{ + g_assert (!user_data); + return G_SOURCE_CONTINUE; +} + /*****************************************************************************/ static inline gboolean From 96b63d3feefd4c7a7c082446846b81f1435248cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 12:55:24 +0100 Subject: [PATCH 02/10] shared/tests: add nmtst_main_context_iterate_until_full() helper (cherry picked from commit 230250e62900be4ba71f90c391e25638f476125e) (cherry picked from commit 20cfc1f395a175675a8b42a2477faa4895de4e1e) (cherry picked from commit c6666589a8672c58086cac828724e34052b65dd3) (cherry picked from commit fa82b663272067cb5174707379fbd4da31067ad3) (cherry picked from commit 08a8b757a38c71c7ff5ad6990d8e3acb1ef17b65) --- shared/nm-utils/nm-test-utils.h | 42 +++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index c3d6447f80..25277b2f76 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1051,19 +1051,32 @@ _nmtst_main_loop_quit_on_notify (GObject *object, GParamSpec *pspec, gpointer us } #define nmtst_main_loop_quit_on_notify ((GCallback) _nmtst_main_loop_quit_on_notify) -#define nmtst_main_context_iterate_until(context, timeout_msec, condition) \ +#define nmtst_main_context_iterate_until_full(context, timeout_msec, poll_msec, condition) \ ({ \ - nm_auto_destroy_and_unref_gsource GSource *_source = NULL; \ + nm_auto_destroy_and_unref_gsource GSource *_source_timeout = NULL; \ + nm_auto_destroy_and_unref_gsource GSource *_source_poll = NULL; \ GMainContext *_context = (context); \ gboolean _had_timeout = FALSE; \ - typeof(timeout_msec) _timeout_msec0 = (timeout_msec); \ + typeof (timeout_msec) _timeout_msec0 = (timeout_msec); \ + typeof (poll_msec) _poll_msec0 = (poll_msec); \ gint64 _timeout_msec = _timeout_msec0; \ + guint _poll_msec = _poll_msec0; \ \ - g_assert_cmpint(_timeout_msec0, ==, _timeout_msec); \ + g_assert_cmpint (_timeout_msec0, ==, _timeout_msec); \ + g_assert_cmpint (_poll_msec0, ==, _poll_msec); \ \ - _source = g_timeout_source_new (NM_CLAMP (_timeout_msec, 0, (gint64) G_MAXUINT)); \ - g_source_set_callback (_source, nmtst_g_source_set_boolean_true, &_had_timeout, NULL); \ - g_source_attach (_source, _context); \ + _source_timeout = g_timeout_source_new (NM_CLAMP (_timeout_msec, 0, (gint64) G_MAXUINT)); \ + g_source_set_callback (_source_timeout, \ + nmtst_g_source_set_boolean_true, \ + &_had_timeout, \ + NULL); \ + g_source_attach (_source_timeout, _context); \ + \ + if (_poll_msec > 0) { \ + _source_poll = g_timeout_source_new (_poll_msec); \ + g_source_set_callback (_source_poll, nmtst_g_source_nop, NULL, NULL); \ + g_source_attach (_source_poll, _context); \ + } \ \ while (TRUE) { \ if (condition) \ @@ -1076,12 +1089,21 @@ _nmtst_main_loop_quit_on_notify (GObject *object, GParamSpec *pspec, gpointer us !_had_timeout; \ }) -#define nmtst_main_context_iterate_until_assert(context, timeout_msec, condition) \ +#define nmtst_main_context_iterate_until(context, timeout_msec, condition) \ + nmtst_main_context_iterate_until_full ((context), (timeout_msec), 0, condition) + +#define nmtst_main_context_iterate_until_assert_full(context, timeout_msec, poll_msec, condition) \ G_STMT_START { \ - if (!nmtst_main_context_iterate_until (context, timeout_msec, condition)) \ - g_assert (FALSE && #condition); \ + if (!nmtst_main_context_iterate_until_full ((context), \ + (timeout_msec), \ + (poll_msec), \ + condition)) \ + g_assert (FALSE &&#condition); \ } G_STMT_END +#define nmtst_main_context_iterate_until_assert(context, timeout_msec, condition) \ + nmtst_main_context_iterate_until_assert_full ((context), (timeout_msec), 0, condition) + /*****************************************************************************/ static inline const char * From 0958a291b09c6cc4e8f52357a07ca643996c6c35 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 12:58:29 +0100 Subject: [PATCH 03/10] tests: avoid race condition in nmtstc_service_cleanup() It seems it can happen that the service is not yet unregistered from the D-Bus broker, even if we already reaped the PID. /builds/NetworkManager/NetworkManager/tools/run-nm-test.sh --called-from-make /builds/NetworkManager/NetworkManager/build --launch-dbus=auto /builds/NetworkManager/NetworkManager/build/libnm/tests/test-nm-client --- stdout --- /libnm/device-added: nmtst: initialize nmtst_get_rand() with NMTST_SEED_RAND=0 --- stderr --- ** test:ERROR:../shared/nm-test-utils-impl.c:216:nmtstc_service_cleanup: assertion failed: (!name_exists(info->bus, "org.freedesktop.NetworkManager")) Workaround by waiting a bit. We now iterate the main GMainContext, unlike before. But that should not cause any problems for the test. (cherry picked from commit 1b8ccacc5da943f6c460d06f42cceb00f390b2b0) (cherry picked from commit d10d14d7ba2bfc404e3862277fcc47f10b84255f) (cherry picked from commit d34e6193da7ec23da9c254c821ec8b4583c65be1) (cherry picked from commit b4302981330995ef18f617df1d63a0d348760de8) (cherry picked from commit 2009025c467175f90334c1aa9206624f319d7c4e) --- shared/nm-test-utils-impl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shared/nm-test-utils-impl.c b/shared/nm-test-utils-impl.c index eca037ba98..fbf62d5ffe 100644 --- a/shared/nm-test-utils-impl.c +++ b/shared/nm-test-utils-impl.c @@ -219,7 +219,10 @@ again_wait: g_assert (ret == info->pid); } - g_assert (!name_exists (info->bus, "org.freedesktop.NetworkManager")); + nmtst_main_context_iterate_until_assert_full (NULL, + 1000, + 80, + (!name_exists (info->bus, "org.freedesktop.NetworkManager"))); g_clear_object (&info->bus); From bb31dcd5efa09a4990a77ed191407e24b2e08861 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 09:12:52 +0100 Subject: [PATCH 04/10] build: don't depend dispatcher code on introspection sources The dispatcher code does not use the generated introspection sources (anymore). Don't add a dependency. (cherry picked from commit e0a3a5e2f82f2dc2b802c91b7cfa2d86f5e093d6) (cherry picked from commit 6ba600cb38ba94724ed7e45c496c254ddd1e24ff) (cherry picked from commit b8a172a6228deab4d18ee666a4c9a4d636f8c479) (cherry picked from commit 2d6690ecac58d01e7a7372d0ff47181aa7182847) (cherry picked from commit 9b0ee443ef6110119374497f8815993fd01f3c15) --- Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4ad4634e13..cc87570487 100644 --- a/Makefile.am +++ b/Makefile.am @@ -738,8 +738,6 @@ dbusinterfaces_DATA = \ CLEANFILES += $(introspection_sources) CLEANFILES += $(DBUS_INTERFACE_DOCS) -$(dispatcher_libnm_dispatcher_core_la_OBJECTS): $(introspection_sources) -$(dispatcher_nm_dispatcher_OBJECTS): $(introspection_sources) $(libnm_libnm_la_OBJECTS): $(introspection_sources) EXTRA_DIST += \ From 5c317a86e2acda58f9ea52db3decac09cd633bae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 09:12:10 +0100 Subject: [PATCH 05/10] introspection: avoid compiler warning in generated introspection files Disable "-Wincompatible-pointer-types-discards-qualifiers" warning, as this breaks build of the gdbus-codegen files. With glib2-2.67.0-1.fc34.x86_64.rpm, clang-11.0.0-2.fc34.x86_64.rpm, we get a failure to build generated code: introspection/org.freedesktop.NetworkManager.AccessPoint.c:438:1: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] G_DEFINE_INTERFACE (NMDBusAccessPoint, nmdbus_access_point, G_TYPE_OBJECT) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:1784:47: note: expanded from macro 'G_DEFINE_INTERFACE' #define G_DEFINE_INTERFACE(TN, t_n, T_P) G_DEFINE_INTERFACE_WITH_CODE(TN, t_n, T_P, ;) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:1803:61: note: expanded from macro 'G_DEFINE_INTERFACE_WITH_CODE' #define G_DEFINE_INTERFACE_WITH_CODE(TN, t_n, T_P, _C_) _G_DEFINE_INTERFACE_EXTENDED_BEGIN(TN, t_n, T_P) {_C_;} _G_DEFINE_INTERFACE_EXTENDED_END() ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:2042:7: note: expanded from macro '_G_DEFINE_INTERFACE_EXTENDED_BEGIN' if (g_once_init_enter (&g_define_type_id__volatile)) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ introspection/org.freedesktop.NetworkManager.AccessPoint.c:944:1: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] G_DEFINE_TYPE_WITH_CODE (NMDBusAccessPointProxy, nmdbus_access_point_proxy, G_TYPE_DBUS_PROXY, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:1615:56: note: expanded from macro 'G_DEFINE_TYPE_WITH_CODE' #define G_DEFINE_TYPE_WITH_CODE(TN, t_n, T_P, _C_) _G_DEFINE_TYPE_EXTENDED_BEGIN (TN, t_n, T_P, 0) {_C_;} _G_DEFINE_TYPE_EXTENDED_END() ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:2032:3: note: expanded from macro '_G_DEFINE_TYPE_EXTENDED_BEGIN' _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:2000:7: note: expanded from macro '_G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER' if (g_once_init_enter (&g_define_type_id__volatile)) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ introspection/org.freedesktop.NetworkManager.AccessPoint.c:1729:1: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] G_DEFINE_TYPE_WITH_CODE (NMDBusAccessPointSkeleton, nmdbus_access_point_skeleton, G_TYPE_DBUS_INTERFACE_SKELETON, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:1615:56: note: expanded from macro 'G_DEFINE_TYPE_WITH_CODE' #define G_DEFINE_TYPE_WITH_CODE(TN, t_n, T_P, _C_) _G_DEFINE_TYPE_EXTENDED_BEGIN (TN, t_n, T_P, 0) {_C_;} _G_DEFINE_TYPE_EXTENDED_END() ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:2032:3: note: expanded from macro '_G_DEFINE_TYPE_EXTENDED_BEGIN' _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/gobject/gtype.h:2000:7: note: expanded from macro '_G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER' if (g_once_init_enter (&g_define_type_id__volatile)) \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ (cherry picked from commit 03d9ec27face7f54e15f6fe6532c59f8b66bb6b4) (cherry picked from commit 595c5854ad068ae025b36d36f5204465186a7145) (cherry picked from commit 3a31fc459adbe48016955afc68a7921045ead408) (cherry picked from commit cb28b6a00b4895b647f1e1794fe60f81fda5d141) (cherry picked from commit 983f87c5b25000b0c3e282c6b1612de45764e0bc) --- Makefile.am | 13 +++++++++++-- dispatcher/tests/meson.build | 2 +- introspection/meson.build | 1 + meson.build | 2 ++ src/settings/plugins/ifcfg-rh/meson.build | 1 + 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index cc87570487..2bb9f633f4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -84,6 +84,8 @@ completiondir = $(datadir)/bash-completion/completions nmlocaledir = $(datadir)/locale +INTROSPECTION_EXTRA_CFLAGS = -Wno-incompatible-pointer-types-discards-qualifiers + GLIB_MKENUMS_H_FLAGS = --identifier-prefix NM GLIB_MKENUMS_C_FLAGS = --identifier-prefix NM @@ -516,7 +518,10 @@ shared_nm_utils_tests_test_shared_general_LDADD = \ noinst_LTLIBRARIES += introspection/libnmdbus.la -introspection_libnmdbus_la_CPPFLAGS = $(GLIB_CFLAGS) +introspection_libnmdbus_la_CPPFLAGS = \ + $(GLIB_CFLAGS) \ + $(INTROSPECTION_EXTRA_CFLAGS) \ + $(NULL) introspection_sources = \ introspection/org.freedesktop.NetworkManager.AccessPoint.c \ @@ -2787,7 +2792,10 @@ nodist_src_settings_plugins_ifcfg_rh_libnmdbus_ifcfg_rh_la_SOURCES = \ src/settings/plugins/ifcfg-rh/nmdbus-ifcfg-rh.c \ src/settings/plugins/ifcfg-rh/nmdbus-ifcfg-rh.h -src_settings_plugins_ifcfg_rh_libnmdbus_ifcfg_rh_la_CPPFLAGS = $(filter-out -DGLIB_VERSION_MAX_ALLOWED%,$(src_cppflags_base)) +src_settings_plugins_ifcfg_rh_libnmdbus_ifcfg_rh_la_CPPFLAGS = \ + $(filter-out -DGLIB_VERSION_MAX_ALLOWED%,$(src_cppflags_base)) \ + $(INTROSPECTION_EXTRA_CFLAGS) \ + $(NULL) CLEANFILES += $(nodist_src_settings_plugins_ifcfg_rh_libnmdbus_ifcfg_rh_la_SOURCES) @@ -3972,6 +3980,7 @@ dispatcher_tests_test_dispatcher_envp_CPPFLAGS = \ -DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_CLIENT \ $(GLIB_CFLAGS) \ $(SANITIZER_EXEC_CFLAGS) \ + $(INTROSPECTION_EXTRA_CFLAGS) \ $(NULL) dispatcher_tests_test_dispatcher_envp_SOURCES = \ diff --git a/dispatcher/tests/meson.build b/dispatcher/tests/meson.build index 9d6a16de1f..953926ebf1 100644 --- a/dispatcher/tests/meson.build +++ b/dispatcher/tests/meson.build @@ -16,7 +16,7 @@ exe = executable( c_args: [ '-DNETWORKMANAGER_COMPILATION_TEST', '-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_CLIENT', - ], + ] + introspection_extra_cflags, link_with: libnm_dispatcher_core, ) diff --git a/introspection/meson.build b/introspection/meson.build index ad9a6db0e2..7d6abe0d4b 100644 --- a/introspection/meson.build +++ b/introspection/meson.build @@ -101,6 +101,7 @@ libnmdbus = static_library( sources: sources, include_directories: top_inc, dependencies: glib_dep, + c_args: introspection_extra_cflags, ) libnmdbus_dep = declare_dependency( diff --git a/meson.build b/meson.build index 929680bcaa..086e7e6d2e 100644 --- a/meson.build +++ b/meson.build @@ -54,6 +54,8 @@ nm_pkgstatedir = join_paths(nm_localstatedir, 'lib', nm_name) nm_vpndir = join_paths(nm_libdir, nm_name) nm_plugindir = join_paths(nm_libdir, nm_name, dist_version) +introspection_extra_cflags = ['-Wno-incompatible-pointer-types-discards-qualifiers'] + libnm_name = 'libnm' current = 1 diff --git a/src/settings/plugins/ifcfg-rh/meson.build b/src/settings/plugins/ifcfg-rh/meson.build index 58acdcfcb1..1b79a48cf5 100644 --- a/src/settings/plugins/ifcfg-rh/meson.build +++ b/src/settings/plugins/ifcfg-rh/meson.build @@ -16,6 +16,7 @@ libnmdbus_ifcfg_rh = static_library( name, sources: dbus_sources, dependencies: glib_dep, + c_args: introspection_extra_cflags, ) core_sources = files( From 9654d6fa616bca3e23f6e1a51646e15319ed9938 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 10:08:09 +0100 Subject: [PATCH 06/10] shared: add a compat implementation for g_atomic_pointer_get() With glib2-2.67.0-1.fc34.x86_64.rpm, clang-11.0.0-2.fc34.x86_64.rpm, we get a failure for g_atomic_pointer_get(): ../shared/nm-glib-aux/nm-hash-utils.c:38:9: error: passing 'typeof (*(&global_seed)) *' (aka 'const unsigned char *volatile *') to parameter of type 'const guint8 **' (aka 'const unsigned char **') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] g = g_atomic_pointer_get(&global_seed); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ ../shared/nm-glib-aux/nm-hash-utils.c:109:32: error: passing 'typeof (*(&global_seed)) *' (aka 'const unsigned char *volatile *') to parameter of type 'const guint8 **' (aka 'const unsigned char **') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] return ((*((const guint *) _get_hash_key())) ^ static_seed) ?: 3679500967u; ^~~~~~~~~~~~~~~ ../shared/nm-glib-aux/nm-hash-utils.c:84:14: note: expanded from macro '_get_hash_key' _g = g_atomic_pointer_get(&global_seed); \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ ../shared/nm-glib-aux/nm-hash-utils.c:123:9: error: passing 'typeof (*(&global_seed)) *' (aka 'const unsigned char *volatile *') to parameter of type 'const guint8 **' (aka 'const unsigned char **') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] g = _get_hash_key(); ^~~~~~~~~~~~~~~ ../shared/nm-glib-aux/nm-hash-utils.c:84:14: note: expanded from macro '_get_hash_key' _g = g_atomic_pointer_get(&global_seed); \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ (cherry picked from commit 5e57ea37f0944bfa3441a51afb1bd5e81b8c5b41) (cherry picked from commit 1e4cfba6dc583ebe071506c42f2a04a9915959b0) (cherry picked from commit 446cac18ccb209334644d1347f300599da14c0b9) (cherry picked from commit af4d01b51b282af95e7841d7c02f73ece8cef541) (cherry picked from commit 17c83d09eb11156d5e42fbd56a854f6b4037aa14) --- shared/nm-glib-aux/nm-glib.h | 41 ++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/shared/nm-glib-aux/nm-glib.h b/shared/nm-glib-aux/nm-glib.h index 45f2c50a3c..6e0ccb24a8 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -577,6 +577,30 @@ _nm_g_value_unset (GValue *value) /*****************************************************************************/ +/* g_atomic_pointer_get() is implemented as a macro, and it is also used for + * (gsize *) arguments. However, that leads to compiler warnings in certain + * configurations. Work around it, by redefining the macro. */ +static inline gpointer +_g_atomic_pointer_get (void **atomic) +{ + return g_atomic_pointer_get (atomic); +} +#undef g_atomic_pointer_get +#define g_atomic_pointer_get(atomic) \ + ({ \ + typeof (*atomic) *const _atomic = (atomic); \ + \ + /* g_atomic_pointer_get() is used by glib also for (gsize *) pointers, + * not only pointers to pointers. We thus don't enforce that (*atomic) + * is a pointer, but of suitable size/alignment. */ \ + \ + G_STATIC_ASSERT (sizeof (*_atomic) == sizeof (gpointer)); \ + G_STATIC_ASSERT (_nm_alignof (*_atomic) == _nm_alignof (gpointer)); \ + (void) (0 ? (gpointer) * (_atomic) : NULL); \ + \ + (typeof (*_atomic)) _g_atomic_pointer_get ((void **) _atomic); \ + }) + /* Glib implements g_atomic_pointer_compare_and_exchange() as a macro. * For one, to inline the atomic operation and also to perform some type checks * on the arguments. @@ -585,20 +609,25 @@ _nm_g_value_unset (GValue *value) * pointers there. Reimplement the macro to get that right, but with stronger * type checks (as we use typeof()). Had one job. */ static inline gboolean -_g_atomic_pointer_compare_and_exchange (volatile void *atomic, - gconstpointer oldval, - gconstpointer newval) +_g_atomic_pointer_compare_and_exchange (void **atomic, + void *oldval, + void *newval) { - return g_atomic_pointer_compare_and_exchange ((void **) atomic, (void *) oldval, (void *) newval); + return g_atomic_pointer_compare_and_exchange (atomic, oldval, newval); } #undef g_atomic_pointer_compare_and_exchange #define g_atomic_pointer_compare_and_exchange(atomic, oldval, newval) \ ({ \ - typeof (atomic) const _atomic = (atomic); \ + typeof (*atomic) *const _atomic = (atomic); \ typeof (*_atomic) const _oldval = (oldval); \ typeof (*_atomic) const _newval = (newval); \ + _nm_unused gconstpointer const _val_type_check = _oldval; \ \ - _g_atomic_pointer_compare_and_exchange (_atomic, _oldval, _newval); \ + (void) (0 ? (gpointer) * (_atomic) : NULL); \ + \ + _g_atomic_pointer_compare_and_exchange ((void **) _atomic, \ + (void *) _oldval, \ + (void *) _newval); \ }) /*****************************************************************************/ From 4862953355a67da13c4601f4e92101922fa71c3f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 10:35:25 +0100 Subject: [PATCH 07/10] shared: also reimplement g_atomic_pointer_set() macro It's not strictly necessary, because contrary to g_atomic_pointer_get() and g_atomic_pointer_compare_and_exchange(), glib's variant for the setter is mostly fine. Still, reimplement it, because we use typeof() eagerly and can thus add more static checks than glib. (cherry picked from commit 7c60e984b6fe742f5719550bd7ccad64608ecb6e) (cherry picked from commit 6ded463f36d945658d601970b5089f2f887f4c5b) (cherry picked from commit 976b358be688864716ade41b94b5872ad95ff4f7) (cherry picked from commit 296a770a8588b2437872fcc23e878a50a3f6e8d1) (cherry picked from commit 32c81a29d5f378fe0682db99c1df31bfbba34054) --- shared/nm-glib-aux/nm-glib.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/shared/nm-glib-aux/nm-glib.h b/shared/nm-glib-aux/nm-glib.h index 6e0ccb24a8..ea020796fb 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -601,6 +601,25 @@ _g_atomic_pointer_get (void **atomic) (typeof (*_atomic)) _g_atomic_pointer_get ((void **) _atomic); \ }) +/* Reimplement g_atomic_pointer_set() macro too. Our variant does more type + * checks. */ +static inline void +_g_atomic_pointer_set (void **atomic, void *newval) +{ + return g_atomic_pointer_set (atomic, newval); +} +#undef g_atomic_pointer_set +#define g_atomic_pointer_set(atomic, newval) \ + ({ \ + typeof (*atomic) *const _atomic = (atomic); \ + typeof (*_atomic) const _newval = (newval); \ + _nm_unused gconstpointer const _val_type_check = _newval; \ + \ + (void) (0 ? (gpointer) * (_atomic) : NULL); \ + \ + _g_atomic_pointer_set ((void **) _atomic, (void *) _newval); \ + }) + /* Glib implements g_atomic_pointer_compare_and_exchange() as a macro. * For one, to inline the atomic operation and also to perform some type checks * on the arguments. From 2bfa92a4c6cfcd2d761b8187d186fc12a7b5fe39 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 16:19:42 +0100 Subject: [PATCH 08/10] shared: don't enforce unset G_LOG_DOMAIN in "nm-default.h" When including , it will always define G_LOG_DOMAIN if it is not yet defined. Usually we want to include "nm-default.h" as very first header. In that case, is not yet included. Then the previous check #error works well. However, if we include "nm-default.h" in sources generated by glib-mkenums, then the generator first already includes , and thus defines G_LOG_DOMAIN. It does so for "libnm-core/nm-core-enum-types.c" and "libnm/nm-enum-types.c", where the #error would not trigger. But we will also include "nm-default.h" for "libnm-core/tests/nm-core-tests-enum-types.c". That will start triggering this #error. While in general we want to include "nm-default.h" first, we also need to support cases where gets included first. Thus this error is not useful. Remove it. (cherry picked from commit 42fa8f3d2722dd9fa987b95919f8dd42d23f0367) (cherry picked from commit a1f3cebbec68022f4665dc29aff1b58ef116c2c8) (cherry picked from commit 61d78ed3337d1682ea1ee255a0f7334e0b633342) (cherry picked from commit 33113c718827a261ce1da74e4c6fa06b97f70b6a) (cherry picked from commit cefe7456fdbcccd2204dfa1b2638d70764879278) --- shared/nm-default.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/shared/nm-default.h b/shared/nm-default.h index 16a756c27e..c70bbef9e3 100644 --- a/shared/nm-default.h +++ b/shared/nm-default.h @@ -90,8 +90,6 @@ #else #error Need to define G_LOG_DOMAIN #endif -#elif defined (NETWORKMANAGER_COMPILATION_TEST) || (NETWORKMANAGER_COMPILATION & NM_NETWORKMANAGER_COMPILATION_WITH_DAEMON) -#error Do not define G_LOG_DOMAIN with NM_NETWORKMANAGER_COMPILATION_WITH_DAEMON #endif /*****************************************************************************/ From 87a0904b8182871a369490f936f4954e349a9f75 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 16:25:53 +0100 Subject: [PATCH 09/10] libnm/tests: include "nm-default.h" for "libnm-core/tests/nm-core-tests-enum-types.c" With glib2-2.67.0-1.fc34.x86_64.rpm, clang-11.0.0-2.fc34.x86_64.rpm, the generated code emits a compiler warning: libnm-core/tests/nm-core-tests-enum-types.c:17:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] if (g_once_init_enter (&g_define_type_id__volatile)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ libnm-core/tests/nm-core-tests-enum-types.c:40:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] if (g_once_init_enter (&g_define_type_id__volatile)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ libnm-core/tests/nm-core-tests-enum-types.c:63:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] if (g_once_init_enter (&g_define_type_id__volatile)) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter' (!g_atomic_pointer_get (location) && \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get' __atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \ ^~~~~~~~~~~~~~~~~ We could pass "-Wincompatible-pointer-types-discards-qualifiers" as CFLAGS when building this file. However, we have a workaround in our "nm-glib-aux/nm-glib.h", so we can instead include "nm-default.h". At first glance, that might look like the less preferable solution. However, this file is only there for unit tests, and we also include "nm-default.h" for other sources that are generated with "glib-mkenums". So, doing it also for our tests becomes the preferable solution. (cherry picked from commit 755d97d38c62244e43a29b3d3426448003d1323b) (cherry picked from commit 294efba18f0e3f4beeaa482a0ee426ebd7bcd02c) (cherry picked from commit 2cff3f369b6bc80b346eae1754c90b3de2095636) (cherry picked from commit cb1a632ccf9834ef1328407315ce16722b0fa517) (cherry picked from commit df4008e99b5d61be62e74d2fb951d655cb53316e) --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 2bb9f633f4..79817adc8e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1067,6 +1067,7 @@ GLIB_GENERATED += \ libnm-core/tests/nm-core-tests-enum-types.h \ libnm-core/tests/nm-core-tests-enum-types.c nm_core_tests_enum_types_sources = libnm-core/tests/test-general-enums.h +nm_core_tests_enum_types_MKENUMS_C_FLAGS = --fhead '\#include "nm-default.h"\n' libnm-core/tests/nm-core-tests-enum-types.h.stamp: libnm-core/tests/.dirstamp libnm-core/tests/nm-core-tests-enum-types.c.stamp: libnm-core/tests/.dirstamp From 8d94c7e291e36f06febd98772eca32ac939e1ab5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Nov 2020 23:34:37 +0100 Subject: [PATCH 10/10] libnm: with meson include "nm-default.h" for glib-mkenums sources We also do that for the autotools implementation. (cherry picked from commit fd57e9665cea3547ce63ad3d693d956cb8c286c7) (cherry picked from commit c807e77271af7972abcecd637cae85e7664cbcc9) (cherry picked from commit 63f2d73b970bba7d2b9ff5e67715e0aefe7fb2d2) (cherry picked from commit 6efb6696c7af7f2b7ae3ff007144ad22cb4378d5) (cherry picked from commit ea9c19ec9d43ecb533c46081140f5453c6f61a0f) --- libnm-core/nm-core-enum-types.c.template | 2 +- libnm-core/tests/nm-core-tests-enum-types.c.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-core-enum-types.c.template b/libnm-core/nm-core-enum-types.c.template index b6cb38eecc..812e1eae19 100644 --- a/libnm-core/nm-core-enum-types.c.template +++ b/libnm-core/nm-core-enum-types.c.template @@ -1,5 +1,5 @@ /*** BEGIN file-header ***/ -#include "config.h" +#include "nm-default.h" #include "nm-core-enum-types.h" #include "nm-default.h" diff --git a/libnm-core/tests/nm-core-tests-enum-types.c.template b/libnm-core/tests/nm-core-tests-enum-types.c.template index 1160be86dc..dab286a3dd 100644 --- a/libnm-core/tests/nm-core-tests-enum-types.c.template +++ b/libnm-core/tests/nm-core-tests-enum-types.c.template @@ -1,5 +1,5 @@ /*** BEGIN file-header ***/ -#include "config.h" +#include "nm-default.h" #include "nm-core-tests-enum-types.h"