From 2d59d02d3354bf6cacbad6627940d664718fdc89 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) --- 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 5a689c825b..4383cf256f 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -985,6 +985,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 08a8b757a38c71c7ff5ad6990d8e3acb1ef17b65 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) --- 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 4383cf256f..5bc0cb27af 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1037,19 +1037,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) \ @@ -1062,12 +1075,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 2009025c467175f90334c1aa9206624f319d7c4e 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) --- 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 0805d0b946..ea53db1408 100644 --- a/shared/nm-test-utils-impl.c +++ b/shared/nm-test-utils-impl.c @@ -205,7 +205,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 9b0ee443ef6110119374497f8815993fd01f3c15 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) --- Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 21d05788e1..4130ff44f7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -782,8 +782,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_liblibnm_la_OBJECTS): $(introspection_sources) $(libnm_libnm_la_OBJECTS): $(introspection_sources) From 983f87c5b25000b0c3e282c6b1612de45764e0bc 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) --- Makefile.am | 13 +++++++++++-- dispatcher/tests/meson.build | 20 ++++++++------------ introspection/meson.build | 1 + meson.build | 2 ++ src/settings/plugins/ifcfg-rh/meson.build | 1 + 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4130ff44f7..6b09071dd6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -85,6 +85,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 @@ -560,7 +562,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 \ @@ -2873,7 +2878,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) @@ -4119,6 +4127,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 83c915e585..66b063f097 100644 --- a/dispatcher/tests/meson.build +++ b/dispatcher/tests/meson.build @@ -1,21 +1,17 @@ test_unit = 'test-dispatcher-envp' -deps = [ - libnm_nm_default_dep, - libnm_utils_base_dep, -] - -c_flags = [ - '-DNETWORKMANAGER_COMPILATION_TEST', - '-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_CLIENT', -] - exe = executable( test_unit, [test_unit + '.c', nmdbus_dispatcher_sources], include_directories: dispatcher_inc, - dependencies: deps, - c_args: c_flags, + dependencies: [ + libnm_nm_default_dep, + libnm_utils_base_dep, + ], + 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 3f8a1b1581..9553491b3e 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 1fb340bb8a..550cc6c695 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 d9dd3edba5..d4b2f7bc7c 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 17c83d09eb11156d5e42fbd56a854f6b4037aa14 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) --- 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 30bd7ea77d..0fe98e40c7 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -581,6 +581,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. @@ -589,20 +613,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 32c81a29d5f378fe0682db99c1df31bfbba34054 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) --- 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 0fe98e40c7..5b2e017dd4 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -605,6 +605,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 cefe7456fdbcccd2204dfa1b2638d70764879278 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) --- shared/nm-default.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/shared/nm-default.h b/shared/nm-default.h index ace6ede16c..c477f6cffd 100644 --- a/shared/nm-default.h +++ b/shared/nm-default.h @@ -75,8 +75,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 df4008e99b5d61be62e74d2fb951d655cb53316e 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) --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 6b09071dd6..1bfe2459f4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1112,6 +1112,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 ea9c19ec9d43ecb533c46081140f5453c6f61a0f 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) --- libnm-core/meson.build | 1 + libnm-core/tests/meson.build | 1 + 2 files changed, 2 insertions(+) diff --git a/libnm-core/meson.build b/libnm-core/meson.build index 469d4161f8..214d49a5e9 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -79,6 +79,7 @@ libnm_core_enum_sources = gnome.mkenums_simple( enum_types, sources: libnm_core_headers + [nm_version_macro_header], identifier_prefix: nm_id_prefix, + body_prefix: '#include "nm-default.h"', install_header: true, install_dir: libnm_pkgincludedir, ) diff --git a/libnm-core/tests/meson.build b/libnm-core/tests/meson.build index 292eeaa015..9f5e179fdb 100644 --- a/libnm-core/tests/meson.build +++ b/libnm-core/tests/meson.build @@ -13,6 +13,7 @@ enum_sources = gnome.mkenums_simple( enum_types, sources: 'test-general-enums.h', identifier_prefix: nm_id_prefix, + body_prefix: '#include "nm-default.h"', ) deps = [