From ca2b79d9aa9999bdb09b31599a4bd942122b282e 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 --- 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 6c945b1920..a6a5c5d171 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1143,6 +1143,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 230250e62900be4ba71f90c391e25638f476125e 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 --- shared/nm-utils/nm-test-utils.h | 80 +++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index a6a5c5d171..42b584c982 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1206,39 +1206,61 @@ _nmtst_main_loop_quit_on_notify(GObject *object, GParamSpec *pspec, gpointer use } #define nmtst_main_loop_quit_on_notify ((GCallback) _nmtst_main_loop_quit_on_notify) -#define nmtst_main_context_iterate_until(context, timeout_msec, condition) \ - ({ \ - nm_auto_destroy_and_unref_gsource GSource *_source = NULL; \ - GMainContext * _context = (context); \ - gboolean _had_timeout = FALSE; \ - typeof(timeout_msec) _timeout_msec0 = (timeout_msec); \ - gint64 _timeout_msec = _timeout_msec0; \ - \ - g_assert_cmpint(_timeout_msec0, ==, _timeout_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); \ - \ - while (TRUE) { \ - if (condition) \ - break; \ - g_main_context_iteration(_context, TRUE); \ - if (_had_timeout) \ - break; \ - } \ - \ - !_had_timeout; \ +#define nmtst_main_context_iterate_until_full(context, timeout_msec, poll_msec, condition) \ + ({ \ + 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(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(_poll_msec0, ==, _poll_msec); \ + \ + _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) \ + break; \ + g_main_context_iteration(_context, TRUE); \ + if (_had_timeout) \ + break; \ + } \ + \ + !_had_timeout; \ }) -#define nmtst_main_context_iterate_until_assert(context, timeout_msec, condition) \ - G_STMT_START \ - { \ - if (!nmtst_main_context_iterate_until(context, timeout_msec, condition)) \ - g_assert(FALSE &&#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_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 1b8ccacc5da943f6c460d06f42cceb00f390b2b0 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. --- shared/nm-test-utils-impl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shared/nm-test-utils-impl.c b/shared/nm-test-utils-impl.c index 0e0d97a494..5ea4968a85 100644 --- a/shared/nm-test-utils-impl.c +++ b/shared/nm-test-utils-impl.c @@ -213,7 +213,11 @@ 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 e0a3a5e2f82f2dc2b802c91b7cfa2d86f5e093d6 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. --- Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 9138f96930..620af4faf1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -903,8 +903,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 03d9ec27face7f54e15f6fe6532c59f8b66bb6b4 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); \ ^~~~~~~~~~~~~~~~~ --- 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 620af4faf1..e95cfddacb 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 @@ -677,7 +679,10 @@ endif 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 \ @@ -3023,7 +3028,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) @@ -4318,6 +4326,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 9136d4126e..6ee66ab01c 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 5e57ea37f0944bfa3441a51afb1bd5e81b8c5b41 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); \ ^~~~~~~~~~~~~~~~~ --- shared/nm-glib-aux/nm-glib.h | 51 +++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/shared/nm-glib-aux/nm-glib.h b/shared/nm-glib-aux/nm-glib.h index 7330b007a8..cb8969ffac 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -586,6 +586,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. @@ -594,22 +618,23 @@ _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 _oldval = (oldval); \ - typeof(*_atomic) const _newval = (newval); \ - \ - _g_atomic_pointer_compare_and_exchange(_atomic, _oldval, _newval); \ +#define g_atomic_pointer_compare_and_exchange(atomic, oldval, newval) \ + ({ \ + typeof(*atomic) *const _atomic = (atomic); \ + typeof(*_atomic) const _oldval = (oldval); \ + typeof(*_atomic) const _newval = (newval); \ + _nm_unused gconstpointer const _val_type_check = _oldval; \ + \ + (void) (0 ? (gpointer) * (_atomic) : NULL); \ + \ + _g_atomic_pointer_compare_and_exchange((void **) _atomic, \ + (void *) _oldval, \ + (void *) _newval); \ }) /*****************************************************************************/ From 7c60e984b6fe742f5719550bd7ccad64608ecb6e 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. --- 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 cb8969ffac..00d9261df7 100644 --- a/shared/nm-glib-aux/nm-glib.h +++ b/shared/nm-glib-aux/nm-glib.h @@ -610,6 +610,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 42fa8f3d2722dd9fa987b95919f8dd42d23f0367 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. --- shared/nm-default.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/shared/nm-default.h b/shared/nm-default.h index 6338c8b8a6..b322f1d37d 100644 --- a/shared/nm-default.h +++ b/shared/nm-default.h @@ -73,9 +73,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 755d97d38c62244e43a29b3d3426448003d1323b 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. --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index e95cfddacb..6e5b9446af 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1220,6 +1220,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 fd57e9665cea3547ce63ad3d693d956cb8c286c7 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. --- 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 a5994db98c..aa8823c991 100644 --- a/libnm-core/meson.build +++ b/libnm-core/meson.build @@ -83,6 +83,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 = [