From e069982492440203b9b7b4982d8904146c85c0b9 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) --- 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 1f39f1443e..e15c31257d 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1056,6 +1056,13 @@ nmtst_g_source_assert_not_called (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 nmtst_g_source_set_boolean_true (gpointer user_data) { From c6666589a8672c58086cac828724e34052b65dd3 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) --- 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 e15c31257d..bf7591d3ad 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1119,19 +1119,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) \ @@ -1144,12 +1157,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 void From d34e6193da7ec23da9c254c821ec8b4583c65be1 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) --- 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 ce7cc8d179..5fbb701812 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 b8a172a6228deab4d18ee666a4c9a4d636f8c479 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) --- Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7ca2ae2216..c3ac237eab 100644 --- a/Makefile.am +++ b/Makefile.am @@ -867,8 +867,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 3a31fc459adbe48016955afc68a7921045ead408 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) --- 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 c3ac237eab..2cf56efe4b 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 @@ -641,7 +643,10 @@ shared_nm_glib_aux_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 \ @@ -2986,7 +2991,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) @@ -4273,6 +4281,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 5c6f6b12a7..53108e79ea 100644 --- a/dispatcher/tests/meson.build +++ b/dispatcher/tests/meson.build @@ -2,22 +2,18 @@ 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 dc7841799c..cc8dcefece 100644 --- a/introspection/meson.build +++ b/introspection/meson.build @@ -104,6 +104,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 dee72a372d..9fc0a712f6 100644 --- a/meson.build +++ b/meson.build @@ -56,6 +56,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 e193ff96a3..10b93d0a7c 100644 --- a/src/settings/plugins/ifcfg-rh/meson.build +++ b/src/settings/plugins/ifcfg-rh/meson.build @@ -18,6 +18,7 @@ libnmdbus_ifcfg_rh = static_library( name, sources: dbus_sources, dependencies: glib_dep, + c_args: introspection_extra_cflags, ) core_sources = files( From 446cac18ccb209334644d1347f300599da14c0b9 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) --- 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 2df5179566..5cac30e12a 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -565,6 +565,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. @@ -573,20 +597,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 976b358be688864716ade41b94b5872ad95ff4f7 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) --- 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 5cac30e12a..bc0f596061 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -589,6 +589,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 61d78ed3337d1682ea1ee255a0f7334e0b633342 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) --- 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 2cff3f369b6bc80b346eae1754c90b3de2095636 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) --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 2cf56efe4b..bef9a22b4f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1195,6 +1195,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 63f2d73b970bba7d2b9ff5e67715e0aefe7fb2d2 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) --- 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 89acaf72ad..d0118faba3 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -82,6 +82,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 58615e4df0..4ce42526df 100644 --- a/libnm-core/tests/meson.build +++ b/libnm-core/tests/meson.build @@ -15,6 +15,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 = [