From 86352ceaf854e6861d370adb28f878c6c746fabb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Oct 2019 12:27:30 +0200 Subject: [PATCH 01/22] shared: add NM_STRUCT_OFFSET_ENSURE_TYPE() helper --- shared/nm-glib-aux/nm-macros-internal.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h index 52441cf14..ea54f4488 100644 --- a/shared/nm-glib-aux/nm-macros-internal.h +++ b/shared/nm-glib-aux/nm-macros-internal.h @@ -635,6 +635,13 @@ NM_G_ERROR_MSG (GError *error) #define _NM_ENSURE_TYPE_CONST(type, value) ((const type) (value)) #endif +#if _NM_CC_SUPPORT_GENERIC +#define NM_STRUCT_OFFSET_ENSURE_TYPE(type, container, field) (_Generic ((((container *) NULL)->field), \ + type: G_STRUCT_OFFSET (container, field))) +#else +#define NM_STRUCT_OFFSET_ENSURE_TYPE(type, container, field) G_STRUCT_OFFSET (container, field) +#endif + #if _NM_CC_SUPPORT_GENERIC /* these macros cast (value) to * - "const char **" (for "MC", mutable-const) From d21439eaa0f80512dce211c9b2bc9032e0873820 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 27 Oct 2019 09:23:39 +0100 Subject: [PATCH 02/22] shared: add nm_ppdirect_hash()/nm_ppdirect_equal() helpers Useful for hashing pointers to a pointer to a pointer, and compare the resulting pointer directly. This will be actually used. --- shared/nm-glib-aux/nm-hash-utils.c | 31 ++++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-hash-utils.h | 10 +++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-hash-utils.c b/shared/nm-glib-aux/nm-hash-utils.c index 8fef015f8..a82125981 100644 --- a/shared/nm-glib-aux/nm-hash-utils.c +++ b/shared/nm-glib-aux/nm-hash-utils.c @@ -206,3 +206,34 @@ nm_pdirect_equal (gconstpointer a, gconstpointer b) && s2 && *s1 == *s2); } + +guint +nm_ppdirect_hash (gconstpointer p) +{ + const void *const*const*s = p; + + if (!s) + return nm_hash_static (396534869u); + if (!*s) + return nm_hash_static (1476102263u); + return nm_direct_hash (**s); +} + +gboolean +nm_ppdirect_equal (gconstpointer a, gconstpointer b) +{ + const void *const*const*s1 = a; + const void *const*const*s2 = b; + + if (s1 == s2) + return TRUE; + if (!s1 || !s2) + return FALSE; + + if (*s1 == *s2) + return TRUE; + if (!*s1 || !*s2) + return FALSE; + + return **s1 == **s2; +} diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index 2e7b35183..c95cc6db9 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -279,12 +279,20 @@ gboolean nm_pstr_equal (gconstpointer a, gconstpointer b); /*****************************************************************************/ /* this hashes/compares the pointer value that we point to. Basically, - * (((const void *const*) a) == ((const void *const*) b)). */ + * (*((const void *const*) a) == *((const void *const*) b)). */ guint nm_pdirect_hash (gconstpointer p); gboolean nm_pdirect_equal (gconstpointer a, gconstpointer b); +/* this hashes/compares the direct pointer value by following pointers to + * pointers 2 times. + * (**((const void *const*const*) a) == **((const void *const*const*) b)). */ + +guint nm_ppdirect_hash (gconstpointer p); + +gboolean nm_ppdirect_equal (gconstpointer a, gconstpointer b); + /*****************************************************************************/ #define NM_HASH_OBFUSCATE_PTR_FMT "%016" G_GINT64_MODIFIER "x" From 9b2d5742c1ca16e3c58cf9c6015c99c1f051a312 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 27 Oct 2019 12:06:20 +0100 Subject: [PATCH 03/22] shared: add nm_g_set_error_take*() util --- shared/nm-glib-aux/nm-shared-utils.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index c027769aa..380c7b766 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -684,6 +684,30 @@ _nm_g_slice_free_fcn_define (32) /*****************************************************************************/ +static inline void +nm_g_set_error_take (GError **error, GError *error_take) +{ + if (!error_take) + g_return_if_reached (); + if (!error) { + g_error_free (error_take); + return; + } + if (*error) { + g_error_free (error_take); + g_return_if_reached (); + } + *error = error_take; +} + +#define nm_g_set_error_take_lazy(error, error_take_lazy) \ + G_STMT_START { \ + GError **_error = (error); \ + \ + if (_error) \ + nm_g_set_error_take (_error, (error_take_lazy)); \ + } G_STMT_END + /** * NMUtilsError: * @NM_UTILS_ERROR_UNKNOWN: unknown or unclassified error From 15fe8ad85126eeab2bf92ffbdd7258947dcd67c0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Oct 2019 12:27:15 +0200 Subject: [PATCH 04/22] shared: add nm_dbus_path_not_empty() helper --- shared/nm-glib-aux/nm-macros-internal.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h index ea54f4488..81b56b583 100644 --- a/shared/nm-glib-aux/nm-macros-internal.h +++ b/shared/nm-glib-aux/nm-macros-internal.h @@ -1268,6 +1268,17 @@ nm_clear_g_cancellable_disconnect (GCancellable *cancellable, gulong *cancellabl /*****************************************************************************/ +static inline const char * +nm_dbus_path_not_empty (const char *str) +{ + nm_assert (!str || str[0] == '/'); + return !str || (str[0] == '/' && str[1] == '\0') + ? NULL + : str; +} + +/*****************************************************************************/ + /* GVariantType is basically a C string. But G_VARIANT_TYPE() is not suitable * to initialize a static variable (because it evaluates a function check that * the string is valid). Add an alternative macro that does the plain cast. From 01335ecec6c95042e43729b27694c88cd725c413 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 29 Oct 2019 16:48:20 +0100 Subject: [PATCH 05/22] shared: assert in nm_ref_string_unref() for valid NMRefString instance --- shared/nm-glib-aux/nm-ref-string.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/shared/nm-glib-aux/nm-ref-string.c b/shared/nm-glib-aux/nm-ref-string.c index f128d5583..0a0b0d3a4 100644 --- a/shared/nm-glib-aux/nm-ref-string.c +++ b/shared/nm-glib-aux/nm-ref-string.c @@ -180,8 +180,14 @@ _nm_ref_string_unref_non_null (NMRefString *rstr) /* in the fast-path above, we already decremented the ref-count to zero. * We need recheck that the ref-count is still zero. */ - if (g_atomic_int_get (&rstr0->ref_count) == 0) - g_hash_table_remove (gl_hash, rstr0); + if (g_atomic_int_get (&rstr0->ref_count) == 0) { + if (!g_hash_table_remove (gl_hash, rstr0)) + nm_assert_not_reached (); + } else { +#if NM_MORE_ASSERTS > 5 + nm_assert (g_hash_table_lookup (gl_hash, rstr0) == rstr0); +#endif + } G_UNLOCK (gl_lock); } From 3b95905ae325db5927ac06ca5eae0fc47afb829f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 2 Nov 2019 15:36:41 +0100 Subject: [PATCH 06/22] shared: add nm_auto_destroy_and_unref_gsource macro --- shared/nm-glib-aux/nm-shared-utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 380c7b766..2ffb97274 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -925,6 +925,9 @@ nm_g_source_destroy_and_unref (GSource *source) g_source_unref (source); } +NM_AUTO_DEFINE_FCN0 (GSource *, _nm_auto_destroy_and_unref_gsource, nm_g_source_destroy_and_unref); +#define nm_auto_destroy_and_unref_gsource nm_auto(_nm_auto_destroy_and_unref_gsource) + /*****************************************************************************/ static inline int From afa54fdfd530392c1c50d578d5366b999a60eef1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Nov 2019 09:26:33 +0100 Subject: [PATCH 07/22] shared: add nm_g_main_context_push_thread_default*() and nm_auto helper --- shared/nm-glib-aux/nm-shared-utils.h | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 2ffb97274..5f1edcb21 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -928,6 +928,42 @@ nm_g_source_destroy_and_unref (GSource *source) NM_AUTO_DEFINE_FCN0 (GSource *, _nm_auto_destroy_and_unref_gsource, nm_g_source_destroy_and_unref); #define nm_auto_destroy_and_unref_gsource nm_auto(_nm_auto_destroy_and_unref_gsource) +NM_AUTO_DEFINE_FCN0 (GMainContext *, _nm_auto_pop_gmaincontext, g_main_context_pop_thread_default) +#define nm_auto_pop_gmaincontext nm_auto (_nm_auto_pop_gmaincontext) + +static inline GMainContext * +nm_g_main_context_push_thread_default (GMainContext *context) +{ + /* This function is to work together with nm_auto_pop_gmaincontext. */ + if (G_UNLIKELY (!context)) + context = g_main_context_default (); + g_main_context_push_thread_default (context); + return context; +} + +static inline GMainContext * +nm_g_main_context_push_thread_default_if_necessary (GMainContext *context) +{ + GMainContext *cur_context; + + cur_context = g_main_context_get_thread_default (); + if (cur_context == context) + return NULL; + + if (G_UNLIKELY (!cur_context)) { + cur_context = g_main_context_default (); + if (cur_context == context) + return NULL; + } else if (G_UNLIKELY (!context)) { + context = g_main_context_default (); + if (cur_context == context) + return NULL; + } + + g_main_context_push_thread_default (context); + return context; +} + /*****************************************************************************/ static inline int From 88ef02ec332b6dc1e94f4831ecb5f2e2d0c68033 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 2 Nov 2019 15:25:30 +0100 Subject: [PATCH 08/22] shared/tests: add nmtst_main_context_iterate_until() helper --- shared/nm-utils/nm-test-utils.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index d4b9578ad..f0ea11790 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1010,6 +1010,34 @@ _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) +static inline gboolean +_nmtst_main_context_iterate_until_timeout (gpointer user_data) +{ + gboolean *p_had_pointer = user_data; + + g_assert (!*p_had_pointer); + *p_had_pointer = TRUE; + return G_SOURCE_CONTINUE; +} + +#define nmtst_main_context_iterate_until(context, timeout_ms, condition) \ + G_STMT_START { \ + nm_auto_destroy_and_unref_gsource GSource *_source = NULL; \ + GMainContext *_context = (context); \ + gboolean _had_timeout = FALSE; \ + \ + _source = g_timeout_source_new (timeout_ms); \ + g_source_set_callback (_source, _nmtst_main_context_iterate_until_timeout, &_had_timeout, NULL); \ + g_source_attach (_source, _context); \ + \ + while (TRUE) { \ + if (condition) \ + break; \ + g_main_context_iteration (_context, TRUE); \ + g_assert (!_had_timeout && #condition); \ + } \ + } G_STMT_END + /*****************************************************************************/ static inline const char * From 1463450393ca7dade3b4131c4fd26dc03861fe6d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Oct 2019 12:29:21 +0200 Subject: [PATCH 09/22] all: use nm_dbus_path_not_empty() --- libnm/nm-libnm-utils.h | 4 +--- src/nm-checkpoint-manager.c | 2 +- src/supplicant/nm-supplicant-interface.c | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/libnm/nm-libnm-utils.h b/libnm/nm-libnm-utils.h index 39b86aac6..ca7d00f54 100644 --- a/libnm/nm-libnm-utils.h +++ b/libnm/nm-libnm-utils.h @@ -124,9 +124,7 @@ _nml_coerce_property_object_path (NMRefString *path) { if (!path) return NULL; - if (nm_streq (path->str, "/")) - return NULL; - return path->str; + return nm_dbus_path_not_empty (path->str); } static inline const char *const* diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c index ded623b3a..8cc8e609e 100644 --- a/src/nm-checkpoint-manager.c +++ b/src/nm-checkpoint-manager.c @@ -209,7 +209,7 @@ nm_checkpoint_manager_destroy (NMCheckpointManager *self, g_return_val_if_fail (path && path[0] == '/', FALSE); g_return_val_if_fail (!error || !*error, FALSE); - if (nm_streq (path, "/")) { + if (!nm_dbus_path_not_empty (path)) { nm_checkpoint_manager_destroy_all (self); return TRUE; } diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index a5e94df60..6ef293112 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -1543,7 +1543,7 @@ p2p_props_changed_cb (GDBusProxy *proxy, if (g_variant_lookup (changed_properties, "Group", "&o", &path)) { if (priv->group_proxy && g_strcmp0 (path, g_dbus_proxy_get_object_path (priv->group_proxy)) == 0) { /* We already have the proxy, nothing to do. */ - } else if (path && g_strcmp0 (path, "/") != 0) { + } else if (nm_dbus_path_not_empty (path)) { if (priv->group_proxy != NULL) { _LOGW ("P2P: Unexpected update of the group object path"); priv->group_proxy_acquired = FALSE; From 9c01d6ca6752e9ad93e570c00754dae72d75028a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 29 Oct 2019 16:50:08 +0100 Subject: [PATCH 10/22] libnm: print timestamp in LIBNM_CLIENT_DEBUG debug logging It's useful, because it's easy to get overwhelemed by the logging output. The timestamp makes it easier to keep track. Also, it allows to see how long things take. --- libnm/nm-libnm-utils.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libnm/nm-libnm-utils.c b/libnm/nm-libnm-utils.c index 7d34d2fd4..3d7b0ba6d 100644 --- a/libnm/nm-libnm-utils.c +++ b/libnm/nm-libnm-utils.c @@ -8,6 +8,8 @@ #include "nm-libnm-utils.h" +#include "nm-glib-aux/nm-time-utils.h" + /*****************************************************************************/ volatile int _nml_dbus_log_level = 0; @@ -43,6 +45,7 @@ _nml_dbus_log (NMLDBusLogLevel level, gs_free char *msg = NULL; va_list args; const char *prefix = ""; + gint64 ts; /* we only call _nml_dbus_log() after nml_dbus_log_enabled(), which already does * an atomic access to the variable. Since the value is only initialized once and @@ -84,7 +87,13 @@ _nml_dbus_log (NMLDBusLogLevel level, break; } - g_printerr ("libnm-dbus: %s%s\n", prefix, msg); + ts = nm_utils_clock_gettime_ns (CLOCK_BOOTTIME); + + g_printerr ("libnm-dbus: %s[%"G_GINT64_FORMAT".%05"G_GINT64_FORMAT"] %s\n", + prefix, + ts / NM_UTILS_NS_PER_SECOND, + (ts / (NM_UTILS_NS_PER_SECOND / 10000)) % 10000, + msg); } /*****************************************************************************/ From 83d7599acce1016c005acaec5af021fc4474c427 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 28 Oct 2019 17:18:39 +0100 Subject: [PATCH 11/22] libnm: deprecate nm_device_set_managed() and nm_device_set_autoconnect() API These setters not only invoke a synchronous D-Bus call (ignoring the return value). They also modify the content of the cache client-side, bypassing the information that we receive via notifications from the server. Also, they don't emit property changed signals, but in any case they are broken beyond repair. Fully mark them as deprecated. Note that they were already marked as _NM_DEPRECATED_SYNC_METHOD. However, that does not actually mark the API as deprecated, because fully deprecating all synchronous methods is premature at this point. --- libnm/nm-device.c | 7 +++++++ libnm/nm-device.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/libnm/nm-device.c b/libnm/nm-device.c index 96698967b..8344f2a03 100644 --- a/libnm/nm-device.c +++ b/libnm/nm-device.c @@ -1066,6 +1066,10 @@ nm_device_get_managed (NMDevice *device) * Since: 1.2 * * Deprecated: 1.22, use nm_device_set_managed_async() or GDBusConnection + * + * This function is deprecated because it calls a synchronous D-Bus method + * and modifies the content of the NMClient cache client side. Also, it does + * not emit a property changed signal. **/ void nm_device_set_managed (NMDevice *device, gboolean managed) @@ -1108,6 +1112,9 @@ nm_device_get_autoconnect (NMDevice *device) * Enables or disables automatic activation of the #NMDevice. * * Deprecated: 1.22, use nm_device_set_autoconnect_async() or GDBusConnection + * + * This function is deprecated because it calls a synchronous D-Bus method + * and modifies the content of the NMClient cache client side. **/ void nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect) diff --git a/libnm/nm-device.h b/libnm/nm-device.h index ede606d8e..bc8aad63d 100644 --- a/libnm/nm-device.h +++ b/libnm/nm-device.h @@ -77,11 +77,13 @@ NMDeviceCapabilities nm_device_get_capabilities (NMDevice *device); gboolean nm_device_get_managed (NMDevice *device); NM_AVAILABLE_IN_1_2 +NM_DEPRECATED_IN_1_22 _NM_DEPRECATED_SYNC_METHOD void nm_device_set_managed (NMDevice *device, gboolean managed); gboolean nm_device_get_autoconnect (NMDevice *device); +NM_DEPRECATED_IN_1_22 _NM_DEPRECATED_SYNC_METHOD void nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect); From 2e91add7e40d7a110acc5de55e41f4aa011beb66 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 29 Oct 2019 16:55:15 +0100 Subject: [PATCH 12/22] libnm: move nm_permission_to_client()/nm_permission_result_to_client() to nm-libnm-utils.c It's nicely trivial and independent. Move it to a separate place, to avoid cluttering the more complicated code and to make it testable. Also, use binary search to find the value by string. --- libnm/nm-libnm-utils.c | 83 ++++++++++++++++++++++++++++++++++++++++++ libnm/nm-libnm-utils.h | 7 ++++ libnm/nm-manager.c | 53 --------------------------- 3 files changed, 90 insertions(+), 53 deletions(-) diff --git a/libnm/nm-libnm-utils.c b/libnm/nm-libnm-utils.c index 3d7b0ba6d..5638d0eae 100644 --- a/libnm/nm-libnm-utils.c +++ b/libnm/nm-libnm-utils.c @@ -9,6 +9,7 @@ #include "nm-libnm-utils.h" #include "nm-glib-aux/nm-time-utils.h" +#include "nm-libnm-core-intern/nm-common-macros.h" /*****************************************************************************/ @@ -658,3 +659,85 @@ nm_utils_fixup_product_string (const char *desc) return desc_full; } + +/*****************************************************************************/ + +NMClientPermission +nm_permission_to_client (const char *nm) +{ + static const struct { + const char *name; + NMClientPermission perm; + } list[] = { + { NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK, NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_CONNECTIVITY_CHECK, NM_CLIENT_PERMISSION_ENABLE_DISABLE_CONNECTIVITY_CHECK }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_NETWORK, NM_CLIENT_PERMISSION_ENABLE_DISABLE_NETWORK }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS, NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_WIFI, NM_CLIENT_PERMISSION_ENABLE_DISABLE_WIFI }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_WIMAX, NM_CLIENT_PERMISSION_ENABLE_DISABLE_WIMAX }, + { NM_AUTH_PERMISSION_ENABLE_DISABLE_WWAN, NM_CLIENT_PERMISSION_ENABLE_DISABLE_WWAN }, + { NM_AUTH_PERMISSION_NETWORK_CONTROL, NM_CLIENT_PERMISSION_NETWORK_CONTROL }, + { NM_AUTH_PERMISSION_RELOAD, NM_CLIENT_PERMISSION_RELOAD }, + { NM_AUTH_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS, NM_CLIENT_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS }, + { NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME, NM_CLIENT_PERMISSION_SETTINGS_MODIFY_HOSTNAME }, + { NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN, NM_CLIENT_PERMISSION_SETTINGS_MODIFY_OWN }, + { NM_AUTH_PERMISSION_SETTINGS_MODIFY_SYSTEM, NM_CLIENT_PERMISSION_SETTINGS_MODIFY_SYSTEM }, + { NM_AUTH_PERMISSION_SLEEP_WAKE, NM_CLIENT_PERMISSION_SLEEP_WAKE }, + { NM_AUTH_PERMISSION_WIFI_SCAN, NM_CLIENT_PERMISSION_WIFI_SCAN }, + { NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, NM_CLIENT_PERMISSION_WIFI_SHARE_OPEN }, + { NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED, NM_CLIENT_PERMISSION_WIFI_SHARE_PROTECTED }, + }; + gssize idx; + +#if NM_MORE_ASSERTS > 10 + { + static gboolean checked = FALSE; + int i, j; + + if (!checked) { + checked = TRUE; + + for (i = 0; i < G_N_ELEMENTS (list); i++) { + + nm_assert (list[i].perm != NM_CLIENT_PERMISSION_NONE); + nm_assert (list[i].name && list[i].name[0]); + if (i > 0) { + if (strcmp (list[i - 1].name, list[i].name) >= 0) { + g_error ("list is not sorted by name: #%d (%s) should be after #%d (%s)", + i - 1, list[i - 1].name, i, list[i].name); + } + } + for (j = i + 1; j < G_N_ELEMENTS (list); j++) { + nm_assert (list[i].perm != list[j].perm); + } + } + } + } +#endif + + if (nm) { + idx = nm_utils_array_find_binary_search (list, + sizeof (list[0]), + G_N_ELEMENTS (list), + &nm, + nm_strcmp_p_with_data, + NULL); + if (idx >= 0) + return list[idx].perm; + } + return NM_CLIENT_PERMISSION_NONE; +} + +NMClientPermissionResult +nm_permission_result_to_client (const char *nm) +{ + if (!nm) + return NM_CLIENT_PERMISSION_RESULT_UNKNOWN; + if (nm_streq (nm, "yes")) + return NM_CLIENT_PERMISSION_RESULT_YES; + if (nm_streq (nm, "no")) + return NM_CLIENT_PERMISSION_RESULT_NO; + if (nm_streq (nm, "auth")) + return NM_CLIENT_PERMISSION_RESULT_AUTH; + return NM_CLIENT_PERMISSION_RESULT_UNKNOWN; +} diff --git a/libnm/nm-libnm-utils.h b/libnm/nm-libnm-utils.h index ca7d00f54..484c00396 100644 --- a/libnm/nm-libnm-utils.h +++ b/libnm/nm-libnm-utils.h @@ -8,6 +8,7 @@ #include "nm-types.h" #include "nm-glib-aux/nm-ref-string.h" +#include "nm-client.h" /*****************************************************************************/ @@ -17,6 +18,12 @@ /*****************************************************************************/ +NMClientPermission nm_permission_to_client (const char *nm); + +NMClientPermissionResult nm_permission_result_to_client (const char *nm); + +/*****************************************************************************/ + typedef enum { _NML_DBUS_LOG_LEVEL_INITIALIZED = 0x01, diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 872c9fa8b..b88aba098 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -254,59 +254,6 @@ object_creation_failed (NMObject *object, const char *failed_path) } } -static NMClientPermission -nm_permission_to_client (const char *nm) -{ - if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_NETWORK)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_NETWORK; - else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_WIFI)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_WIFI; - else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_WWAN)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_WWAN; - else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_WIMAX)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_WIMAX; - else if (!strcmp (nm, NM_AUTH_PERMISSION_SLEEP_WAKE)) - return NM_CLIENT_PERMISSION_SLEEP_WAKE; - else if (!strcmp (nm, NM_AUTH_PERMISSION_NETWORK_CONTROL)) - return NM_CLIENT_PERMISSION_NETWORK_CONTROL; - else if (!strcmp (nm, NM_AUTH_PERMISSION_WIFI_SHARE_PROTECTED)) - return NM_CLIENT_PERMISSION_WIFI_SHARE_PROTECTED; - else if (!strcmp (nm, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN)) - return NM_CLIENT_PERMISSION_WIFI_SHARE_OPEN; - else if (!strcmp (nm, NM_AUTH_PERMISSION_SETTINGS_MODIFY_SYSTEM)) - return NM_CLIENT_PERMISSION_SETTINGS_MODIFY_SYSTEM; - else if (!strcmp (nm, NM_AUTH_PERMISSION_SETTINGS_MODIFY_OWN)) - return NM_CLIENT_PERMISSION_SETTINGS_MODIFY_OWN; - else if (!strcmp (nm, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME)) - return NM_CLIENT_PERMISSION_SETTINGS_MODIFY_HOSTNAME; - else if (!strcmp (nm, NM_AUTH_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS)) - return NM_CLIENT_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS; - else if (!strcmp (nm, NM_AUTH_PERMISSION_RELOAD)) - return NM_CLIENT_PERMISSION_RELOAD; - else if (!strcmp (nm, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK)) - return NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK; - else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS; - else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_CONNECTIVITY_CHECK)) - return NM_CLIENT_PERMISSION_ENABLE_DISABLE_CONNECTIVITY_CHECK; - else if (!strcmp (nm, NM_AUTH_PERMISSION_WIFI_SCAN)) - return NM_CLIENT_PERMISSION_WIFI_SCAN; - - return NM_CLIENT_PERMISSION_NONE; -} - -static NMClientPermissionResult -nm_permission_result_to_client (const char *nm) -{ - if (!strcmp (nm, "yes")) - return NM_CLIENT_PERMISSION_RESULT_YES; - else if (!strcmp (nm, "no")) - return NM_CLIENT_PERMISSION_RESULT_NO; - else if (!strcmp (nm, "auth")) - return NM_CLIENT_PERMISSION_RESULT_AUTH; - return NM_CLIENT_PERMISSION_RESULT_UNKNOWN; -} - static void update_permissions (NMManager *self, GVariant *permissions) { From abb68d90fc8cef0e7a2895b6540b3e766c17648d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 29 Oct 2019 20:05:02 +0100 Subject: [PATCH 13/22] libnm: retire nm_client_wimax_*() functions The server doesn's support WiMAX anymore. Hence there is no point in keeping this functionality. While we cannot drop the functions, let them not do anything. The code in NMManager is still there. But since we will soon drop NMManager entirely, it doesn't matter. --- libnm/nm-client.c | 35 +++++++++++++++-------------------- libnm/nm-client.h | 3 +++ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 0d84923e2..56367eedc 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -511,16 +511,15 @@ nm_client_wwan_hardware_get_enabled (NMClient *client) * Determines whether WiMAX is enabled. * * Returns: %TRUE if WiMAX is enabled + * + * Deprecated: 1.22 This function always returns FALSE because WiMax is no longer supported **/ gboolean nm_client_wimax_get_enabled (NMClient *client) { g_return_val_if_fail (NM_IS_CLIENT (client), FALSE); - if (!nm_client_get_nm_running (client)) - return FALSE; - - return nm_manager_wimax_get_enabled (NM_CLIENT_GET_PRIVATE (client)->manager); + return FALSE; } /** @@ -529,16 +528,13 @@ nm_client_wimax_get_enabled (NMClient *client) * @enabled: %TRUE to enable WiMAX * * Enables or disables WiMAX devices. + * + * Deprecated: 1.22 This function does nothing because WiMax is no longer supported **/ void nm_client_wimax_set_enabled (NMClient *client, gboolean enabled) { g_return_if_fail (NM_IS_CLIENT (client)); - - if (!nm_client_get_nm_running (client)) - return; - - nm_manager_wimax_set_enabled (NM_CLIENT_GET_PRIVATE (client)->manager, enabled); } /** @@ -548,16 +544,15 @@ nm_client_wimax_set_enabled (NMClient *client, gboolean enabled) * Determines whether the WiMAX hardware is enabled. * * Returns: %TRUE if the WiMAX hardware is enabled + * + * Deprecated: 1.22 This function always returns FALSE because WiMax is no longer supported **/ gboolean nm_client_wimax_hardware_get_enabled (NMClient *client) { g_return_val_if_fail (NM_IS_CLIENT (client), FALSE); - if (!nm_client_get_nm_running (client)) - return FALSE; - - return nm_manager_wimax_hardware_get_enabled (NM_CLIENT_GET_PRIVATE (client)->manager); + return FALSE; } /** @@ -3540,13 +3535,10 @@ get_property (GObject *object, guint prop_id, g_value_set_boolean (value, FALSE); break; case PROP_WIMAX_ENABLED: - g_value_set_boolean (value, nm_client_wimax_get_enabled (self)); + g_value_set_boolean (value, FALSE); break; case PROP_WIMAX_HARDWARE_ENABLED: - if (priv->manager) - g_object_get_property (G_OBJECT (priv->manager), pspec->name, value); - else - g_value_set_boolean (value, FALSE); + g_value_set_boolean (value, FALSE); break; case PROP_ACTIVE_CONNECTIONS: g_value_take_boxed (value, _nm_utils_copy_object_array (nm_client_get_active_connections (self))); @@ -3643,11 +3635,12 @@ set_property (GObject *object, guint prop_id, case PROP_NETWORKING_ENABLED: case PROP_WIRELESS_ENABLED: case PROP_WWAN_ENABLED: - case PROP_WIMAX_ENABLED: case PROP_CONNECTIVITY_CHECK_ENABLED: if (priv->manager) g_object_set_property (G_OBJECT (priv->manager), pspec->name, value); break; + case PROP_WIMAX_ENABLED: + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -4015,7 +4008,7 @@ nm_client_class_init (NMClientClass *client_class) * * Whether WiMAX functionality is enabled. * - * The property setter is a synchronous D-Bus call. This is deprecated since 1.22. + * Deprecated: 1.22: WiMAX is no longer supported and this always returns FALSE. The setter has no effect. */ obj_properties[PROP_WIMAX_ENABLED] = g_param_spec_boolean (NM_CLIENT_WIMAX_ENABLED, "", "", @@ -4027,6 +4020,8 @@ nm_client_class_init (NMClientClass *client_class) * NMClient:wimax-hardware-enabled: * * Whether the WiMAX hardware is enabled. + * + * Deprecated: 1.22: WiMAX is no longer supported and this always returns FALSE. **/ obj_properties[PROP_WIMAX_HARDWARE_ENABLED] = g_param_spec_boolean (NM_CLIENT_WIMAX_HARDWARE_ENABLED, "", "", diff --git a/libnm/nm-client.h b/libnm/nm-client.h index f1822c5ea..41481f11f 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -245,11 +245,14 @@ void nm_client_wwan_set_enabled (NMClient *client, gboolean enabled); gboolean nm_client_wwan_hardware_get_enabled (NMClient *client); +NM_DEPRECATED_IN_1_22 gboolean nm_client_wimax_get_enabled (NMClient *client); +NM_DEPRECATED_IN_1_22 _NM_DEPRECATED_SYNC_METHOD void nm_client_wimax_set_enabled (NMClient *client, gboolean enabled); +NM_DEPRECATED_IN_1_22 gboolean nm_client_wimax_hardware_get_enabled (NMClient *client); NM_AVAILABLE_IN_1_10 From ec39498fc6d70cf6887f08b4dfbc5cff021fc231 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Oct 2019 16:38:36 +0100 Subject: [PATCH 14/22] tests: support D-Bus property Device.StateReason in mock service The device interface (org.freedesktop.NetworkManager.Device) has two properties: "State" and "StateReason". Both of them are supported by NetworkManager for a very long time. Note that "StateReason" is a tuple and also exposes the state along the reason. When reworking libnm, we will ignore the "State" property and only consider "StateReason". The advantage is less code and not using redundant state. This will also work well, because NetworkManager's D-Bus API supports this property for a very long time. However, that would then break the CI tests, because currently "tools/test-networkmanager-service.py" does not expose that property. Add it. --- tools/test-networkmanager-service.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index 19d4baef2..e91b45233 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -672,6 +672,7 @@ PRP_DEVICE_UDI = "Udi" PRP_DEVICE_IFACE = "Interface" PRP_DEVICE_DRIVER = "Driver" PRP_DEVICE_STATE = "State" +PRP_DEVICE_STATE_REASON = "StateReason" PRP_DEVICE_ACTIVE_CONNECTION = "ActiveConnection" PRP_DEVICE_IP4_CONFIG = "Ip4Config" PRP_DEVICE_IP6_CONFIG = "Ip6Config" @@ -700,11 +701,14 @@ class Device(ExportedObj): self.dhcp4_config = None self.dhcp6_config = None + self.prp_state = NM.DeviceState.UNAVAILABLE + props = { PRP_DEVICE_UDI: "/sys/devices/virtual/%s" % (iface), PRP_DEVICE_IFACE: iface, PRP_DEVICE_DRIVER: "virtual", - PRP_DEVICE_STATE: dbus.UInt32(NM.DeviceState.UNAVAILABLE), + PRP_DEVICE_STATE: dbus.UInt32(self.prp_state), + PRP_DEVICE_STATE_REASON: dbus.Struct((dbus.UInt32(self.prp_state), dbus.UInt32(NM.DeviceStateReason.NONE))), PRP_DEVICE_ACTIVE_CONNECTION: ExportedObj.to_path(None), PRP_DEVICE_IP4_CONFIG: ExportedObj.to_path(self.ip4_config), PRP_DEVICE_IP6_CONFIG: ExportedObj.to_path(self.ip6_config), From 8def37dd65ae9dfec8d3faca36665c92ea91828f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 2 Nov 2019 14:15:48 +0100 Subject: [PATCH 15/22] tests: add nmtstc_client_new() helper --- shared/nm-test-libnm-utils.h | 1 + shared/nm-test-utils-impl.c | 148 +++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/shared/nm-test-libnm-utils.h b/shared/nm-test-libnm-utils.h index 76e2a2158..63e3b4c72 100644 --- a/shared/nm-test-libnm-utils.h +++ b/shared/nm-test-libnm-utils.h @@ -67,3 +67,4 @@ void nmtstc_service_update_connection_variant (NMTstcServiceInfo *sinfo, GVariant *connection, gboolean verify_connection); +NMClient *nmtstc_client_new (gboolean allow_iterate_main_context); diff --git a/shared/nm-test-utils-impl.c b/shared/nm-test-utils-impl.c index 39f0a0832..bbe74a965 100644 --- a/shared/nm-test-utils-impl.c +++ b/shared/nm-test-utils-impl.c @@ -406,3 +406,151 @@ nmtstc_service_update_connection_variant (NMTstcServiceInfo *sinfo, g_assert (g_variant_is_of_type (result, G_VARIANT_TYPE ("()"))); g_variant_unref (result); } + +/*****************************************************************************/ + +typedef struct { + GMainLoop *loop; + NMClient *client; +} NMTstcClientNewData; + +static void +_nmtstc_client_new_cb (GObject *source_object, + GAsyncResult *res, + gpointer user_data) +{ + NMTstcClientNewData *d = user_data; + gs_free_error GError *error = NULL; + + g_assert (!d->client); + + d->client = nm_client_new_finish (res, + nmtst_get_rand_bool () ? &error : NULL); + + nmtst_assert_success (NM_IS_CLIENT (d->client), error); + + g_main_loop_quit (d->loop); +} + +static NMClient * +_nmtstc_client_new (gboolean sync) +{ + gs_free GError *error = NULL; + NMClient *client; + + /* Create a NMClient instance synchronously, and arbitrarily use either + * the sync or async constructor. + * + * Note that the sync and async construct differ in one important aspect: + * the async constructor iterates the current g_main_context_get_thread_default(), + * while the sync constructor does not! Aside from that, both should behave + * pretty much the same way. */ + + if (sync) { + nm_auto_destroy_and_unref_gsource GSource *source = NULL; + + if (nmtst_get_rand_bool ()) { + /* the current main context must not be iterated! */ + source = g_idle_source_new (); + g_source_set_callback (source, nmtst_g_source_assert_not_called, NULL, NULL); + g_source_attach (source, g_main_context_get_thread_default ()); + } + + if (nmtst_get_rand_bool ()) { + gboolean success; + + client = g_object_new (NM_TYPE_CLIENT, NULL); + g_assert (NM_IS_CLIENT (client)); + + success = g_initable_init (G_INITABLE (client), + NULL, + nmtst_get_rand_bool () ? &error : NULL); + nmtst_assert_success (success, error); + } else { + client = nm_client_new (NULL, + nmtst_get_rand_bool () ? &error : NULL); + } + } else { + nm_auto_unref_gmainloop GMainLoop *loop = NULL; + NMTstcClientNewData d = { .loop = NULL, }; + + loop = g_main_loop_new (g_main_context_get_thread_default (), FALSE); + + d.loop = loop; + nm_client_new_async (NULL, + _nmtstc_client_new_cb, + &d); + g_main_loop_run (loop); + g_assert (NM_IS_CLIENT (d.client)); + client = d.client; + } + + nmtst_assert_success (NM_IS_CLIENT (client), error); + return client; +} + +typedef struct { + GMainLoop *loop; + NMClient *client; + bool sync; +} NewSyncInsideDispatchedData; + +static gboolean +_nmtstc_client_new_inside_loop_do (gpointer user_data) +{ + NewSyncInsideDispatchedData *d = user_data; + + g_assert (d->loop); + g_assert (!d->client); + + d->client = nmtstc_client_new (d->sync); + g_main_loop_quit (d->loop); + return G_SOURCE_CONTINUE; +} + +static NMClient * +_nmtstc_client_new_inside_loop (gboolean sync) +{ + GMainContext *context = g_main_context_get_thread_default (); + nm_auto_unref_gmainloop GMainLoop *loop = g_main_loop_new (context, FALSE); + NewSyncInsideDispatchedData d = { + .sync = sync, + .loop = loop, + }; + nm_auto_destroy_and_unref_gsource GSource *source = NULL; + + source = g_idle_source_new (); + g_source_set_callback (source, _nmtstc_client_new_inside_loop_do, &d, NULL); + g_source_attach (source, context); + + g_main_loop_run (loop); + g_assert (NM_IS_CLIENT (d.client)); + return d.client; +} + +NMClient * +nmtstc_client_new (gboolean allow_iterate_main_context) +{ + gboolean inside_loop; + gboolean sync; + + if (!allow_iterate_main_context) { + sync = TRUE; + inside_loop = FALSE; + } else { + /* The caller allows to iterate the main context. That that point, + * we can both use the synchronous and the asynchronous initialization, + * both should yield the same result. Choose one randomly. */ + sync = nmtst_get_rand_bool (); + inside_loop = ((nmtst_get_rand_uint32 () % 3) == 0); + } + + if (inside_loop) { + /* Create the client on an idle handler of the current context. + * In practice, it should make no difference, which this check + * tries to prove. */ + return _nmtstc_client_new_inside_loop (sync); + } + + return _nmtstc_client_new (sync); +} From f21b8781ed7ab3da080ab5736c77cfff6a9c7328 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 2 Nov 2019 14:20:43 +0100 Subject: [PATCH 16/22] tests: use nmtstc_client_new() to create NMClient instance and cleanup tests The advantage of nmtstc_client_new() is that it randomly either uses the synchronous or asynchronous constructor. Of course, both should behave pretty much the same. Hence, this increases our test coverage. --- libnm/tests/test-nm-client.c | 255 ++++++---------------- libnm/tests/test-remote-settings-client.c | 207 ++++++++---------- libnm/tests/test-secret-agent.c | 5 +- 3 files changed, 159 insertions(+), 308 deletions(-) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index f97aa5607..987b63ed9 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -10,8 +10,9 @@ #include "nm-test-libnm-utils.h" -static GMainLoop *loop = NULL; -static NMTstcServiceInfo *sinfo; +static struct { + GMainLoop *loop; +} gl = { }; /*****************************************************************************/ @@ -24,68 +25,6 @@ loop_quit (gpointer user_data) /*****************************************************************************/ -static NMClient * -_nm_client_new_sync (void) -{ - NMClient *client; - guint source_1; - GError *error = NULL; - - source_1 = g_idle_add (nmtst_g_source_assert_not_called, NULL); - - client = g_object_new (NM_TYPE_CLIENT, NULL); - g_assert (NM_IS_CLIENT (client)); - - if (!g_initable_init (G_INITABLE (client), NULL, &error)) - g_assert_not_reached (); - - g_assert_no_error (error); - - nm_clear_g_source (&source_1); - return client; -} - -typedef struct { - GMainLoop *loop; - NMClient *client; -} NewSyncInsideDispatchedData; - -static gboolean -_nm_client_new_sync_inside_dispatched_do (gpointer user_data) -{ - NewSyncInsideDispatchedData *d = user_data; - - g_assert (d->loop); - g_assert (!d->client); - - d->client = _nm_client_new_sync (); - - g_main_loop_quit (d->loop); - return G_SOURCE_CONTINUE; -} - -static NMClient * -_nm_client_new_sync_inside_dispatched (void) -{ - NewSyncInsideDispatchedData d = { }; - guint source_1; - - d.loop = g_main_loop_new (NULL, FALSE); - source_1 = g_idle_add (_nm_client_new_sync_inside_dispatched_do, &d); - - g_main_loop_run (d.loop); - - g_assert (NM_IS_CLIENT (d.client)); - - nm_clear_g_source (&source_1); - - g_main_loop_unref (d.loop); - - return d.client; -} - -/*****************************************************************************/ - static void devices_notify_cb (NMClient *c, GParamSpec *pspec, @@ -109,7 +48,8 @@ devices_notify_cb (NMClient *c, static void test_device_added (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; const GPtrArray *devices; NMDevice *device; gboolean notified = FALSE; @@ -119,8 +59,7 @@ test_device_added (void) if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); devices = nm_client_get_devices (client); g_assert (devices->len == 0); @@ -151,9 +90,6 @@ test_device_added (void) nm_device_delete (device, NULL, &error); g_assert_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_NOT_SOFTWARE); g_clear_error (&error); - - g_object_unref (client); - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } /*****************************************************************************/ @@ -190,6 +126,8 @@ devices_sai_notify_cb (NMClient *c, const GPtrArray *devices; NMDevice *device; + g_assert_cmpstr (pspec->name, ==, "devices"); + devices = nm_client_get_devices (c); g_assert (devices); g_assert_cmpint (devices->len, ==, 1); @@ -205,18 +143,17 @@ devices_sai_notify_cb (NMClient *c, static void test_device_added_signal_after_init (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; const GPtrArray *devices; NMDevice *device; guint result = 0; - GError *error = NULL; sinfo = nmtstc_service_init (); if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); devices = nm_client_get_devices (client); g_assert (devices->len == 0); @@ -253,9 +190,6 @@ test_device_added_signal_after_init (void) device = g_ptr_array_index (devices, 0); g_assert (device); g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); - - g_object_unref (client); - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } /*****************************************************************************/ @@ -355,19 +289,19 @@ wifi_ap_remove_notify_cb (NMDeviceWifi *w, static void test_wifi_ap_added_removed (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; NMDeviceWifi *wifi; - WifiApInfo info = { loop, FALSE, FALSE, 0, 0 }; + WifiApInfo info = { gl.loop, FALSE, FALSE, 0, 0 }; GVariant *ret; GError *error = NULL; - char *expected_path = NULL; + gs_free char *expected_path = NULL; sinfo = nmtstc_service_init (); if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); /*************************************/ /* Add the wifi device */ @@ -406,8 +340,8 @@ test_wifi_ap_added_removed (void) info.quit_count++; /* Wait for libnm to find the AP */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + info.quit_id = g_timeout_add_seconds (5, loop_quit, gl.loop); + g_main_loop_run (gl.loop); g_assert (info.signaled); g_assert (info.notified); @@ -445,8 +379,8 @@ test_wifi_ap_added_removed (void) info.quit_count++; /* Wait for libnm to find the AP */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + info.quit_id = g_timeout_add_seconds (5, loop_quit, gl.loop); + g_main_loop_run (gl.loop); g_assert (info.signaled); g_assert (info.notified); @@ -454,10 +388,6 @@ test_wifi_ap_added_removed (void) g_signal_handlers_disconnect_by_func (wifi, wifi_ap_remove_notify_cb, &info); g_free (info.ap_path); - g_free (expected_path); - - g_object_unref (client); - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } /*****************************************************************************/ @@ -516,26 +446,12 @@ da_devices_notify_cb (NMClient *c, da_check_quit (info); } -static void -new_client_cb (GObject *object, - GAsyncResult *result, - gpointer user_data) -{ - NMClient **out_client = user_data; - GError *error = NULL; - - *out_client = nm_client_new_finish (result, &error); - g_assert_no_error (error); - g_assert (*out_client != NULL); - - g_main_loop_quit (loop); -} - static void test_devices_array (void) { - NMClient *client = NULL; - DaInfo info = { loop }; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; + DaInfo info = { gl.loop }; NMDevice *wlan0, *eth0, *eth1, *device; const GPtrArray *devices; GError *error = NULL; @@ -545,10 +461,7 @@ test_devices_array (void) if (!nmtstc_service_available (sinfo)) return; - /* Make sure that we test the async codepath in at least one test... */ - nm_client_new_async (NULL, new_client_cb, &client); - g_main_loop_run (loop); - g_assert (client != NULL); + client = nmtstc_client_new (TRUE); /*************************************/ /* Add some devices */ @@ -598,8 +511,8 @@ test_devices_array (void) info.quit_count = 2; /* Wait for libnm to notice the changes */ - info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + info.quit_id = g_timeout_add_seconds (5, loop_quit, gl.loop); + g_main_loop_run (gl.loop); g_assert_cmpint (info.quit_count, ==, 0); g_signal_handlers_disconnect_by_func (client, da_device_removed_cb, &info); @@ -617,9 +530,6 @@ test_devices_array (void) device = nm_client_get_device_by_iface (client, "eth1"); g_assert (NM_IS_DEVICE_ETHERNET (device)); g_assert (device == eth1); - - g_object_unref (client); - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } static void @@ -630,20 +540,20 @@ nm_running_changed (GObject *client, int *running_changed = user_data; (*running_changed)++; - g_main_loop_quit (loop); + g_main_loop_quit (gl.loop); } static void test_client_nm_running (void) { + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; gs_unref_object NMClient *client1 = NULL; gs_unref_object NMClient *client2 = NULL; guint quit_id; int running_changed = 0; GError *error = NULL; - client1 = nm_client_new (NULL, &error); - g_assert_no_error (error); + client1 = nmtstc_client_new (TRUE); g_assert (!nm_client_get_nm_running (client1)); g_assert_cmpstr (nm_client_get_version (client1), ==, NULL); @@ -663,8 +573,7 @@ test_client_nm_running (void) if (!nmtstc_service_available (sinfo)) return; - client2 = nm_client_new (NULL, &error); - g_assert_no_error (error); + client2 = nmtstc_client_new (FALSE); /* client2 should know that NM is running, but the previously-created * client1 hasn't gotten the news yet. @@ -674,8 +583,8 @@ test_client_nm_running (void) g_signal_connect (client1, "notify::" NM_CLIENT_NM_RUNNING, G_CALLBACK (nm_running_changed), &running_changed); - quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + quit_id = g_timeout_add_seconds (5, loop_quit, gl.loop); + g_main_loop_run (gl.loop); g_assert_cmpint (running_changed, ==, 1); g_assert (nm_client_get_nm_running (client1)); g_source_remove (quit_id); @@ -685,8 +594,8 @@ test_client_nm_running (void) g_assert (nm_client_get_nm_running (client1)); - quit_id = g_timeout_add_seconds (5, loop_quit, loop); - g_main_loop_run (loop); + quit_id = g_timeout_add_seconds (5, loop_quit, gl.loop); + g_main_loop_run (gl.loop); g_assert_cmpint (running_changed, ==, 2); g_assert (!nm_client_get_nm_running (client1)); g_source_remove (quit_id); @@ -786,18 +695,17 @@ device_ac_changed_cb (GObject *device, static void test_active_connections (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; NMDevice *device; NMConnection *conn; - TestACInfo info = { loop, NULL, 0 }; - GError *error = NULL; + TestACInfo info = { gl.loop, NULL, 0 }; sinfo = nmtstc_service_init (); if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); /* Tell the test service to add a new device */ device = nmtstc_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); @@ -814,30 +722,25 @@ test_active_connections (void) /* Two signals plus activate_cb */ info.remaining = 3; - g_main_loop_run (loop); + g_main_loop_run (gl.loop); g_signal_handlers_disconnect_by_func (client, client_acs_changed_cb, &info); g_signal_handlers_disconnect_by_func (device, device_ac_changed_cb, &info); g_assert (info.ac != NULL); g_object_unref (info.ac); - g_object_unref (client); + g_clear_object (&client); /* Ensure that we can correctly resolve the recursive property link between the * AC and the Device in a newly-created client. */ - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); assert_ac_and_device (client); - g_object_unref (client); + g_clear_object (&client); - client = NULL; - nm_client_new_async (NULL, new_client_cb, &client); - g_main_loop_run (loop); + client = nmtstc_client_new (TRUE); assert_ac_and_device (client); - g_object_unref (client); - - g_clear_pointer (&sinfo, nmtstc_service_cleanup); + g_clear_object (&client); } static void @@ -916,20 +819,19 @@ activate_cb (GObject *object, static void test_activate_virtual (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; NMConnection *conn; NMSettingConnection *s_con; NMSettingVlan *s_vlan; - TestACInfo info = { loop, NULL, 0 }; - TestConnectionInfo conn_info = { loop, NULL }; - GError *error = NULL; + TestACInfo info = { gl.loop, NULL, 0 }; + TestConnectionInfo conn_info = { gl.loop, NULL }; sinfo = nmtstc_service_init (); if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); nmtstc_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); @@ -945,7 +847,7 @@ test_activate_virtual (void) nm_client_add_connection_async (client, conn, TRUE, NULL, add_connection_cb, &conn_info); - g_main_loop_run (loop); + g_main_loop_run (gl.loop); g_object_unref (conn); conn = NM_CONNECTION (conn_info.remote); @@ -965,16 +867,13 @@ test_activate_virtual (void) */ info.remaining = 3; - g_main_loop_run (loop); + g_main_loop_run (gl.loop); g_signal_handlers_disconnect_by_func (client, client_acs_changed_cb, &info); g_signal_handlers_disconnect_by_func (client, client_devices_changed_cb, &info); g_assert (info.ac != NULL); g_object_unref (info.ac); - g_object_unref (client); - - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } static void @@ -991,23 +890,22 @@ activate_failed_cb (GObject *object, g_assert_error (error, NM_CLIENT_ERROR, NM_CLIENT_ERROR_OBJECT_CREATION_FAILED); g_clear_error (&error); - g_main_loop_quit (loop); + g_main_loop_quit (gl.loop); } static void test_activate_failed (void) { - NMClient *client; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; NMDevice *device; - NMConnection *conn; - GError *error = NULL; + gs_unref_object NMConnection *conn = NULL; sinfo = nmtstc_service_init (); if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); device = nmtstc_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); @@ -1017,20 +915,17 @@ test_activate_failed (void) nm_client_add_and_activate_connection_async (client, conn, device, NULL, NULL, activate_failed_cb, NULL); - g_main_loop_run (loop); - - g_object_unref (conn); - g_object_unref (client); - - g_clear_pointer (&sinfo, nmtstc_service_cleanup); + g_main_loop_run (gl.loop); } static void test_device_connection_compatibility (void) { - NMClient *client; - NMDevice *device1, *device2; - NMConnection *conn; + nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; + gs_unref_object NMClient *client = NULL; + gs_unref_object NMConnection *conn = NULL; + NMDevice *device1; + NMDevice *device2; NMSettingWired *s_wired; GError *error = NULL; const char *subchannels[] = { "0.0.8000", "0.0.8001", "0.0.8002", NULL }; @@ -1043,8 +938,7 @@ test_device_connection_compatibility (void) if (!nmtstc_service_available (sinfo)) return; - client = nm_client_new (NULL, &error); - g_assert_no_error (error); + client = nmtstc_client_new (TRUE); /* Create two devices */ device1 = nmtstc_service_add_wired_device (sinfo, client, "eth0", hw_addr1, subchannels); @@ -1092,11 +986,6 @@ test_device_connection_compatibility (void) nm_device_connection_compatible (device1, conn, &error); g_assert_error (error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_INCOMPATIBLE_CONNECTION); g_clear_error (&error); - - g_object_unref (conn); - g_object_unref (client); - - g_clear_pointer (&sinfo, nmtstc_service_cleanup); } /*****************************************************************************/ @@ -1179,9 +1068,7 @@ test_connection_invalid (void) FALSE, &path2); - client = nmtst_get_rand_bool () - ? _nm_client_new_sync () - : _nm_client_new_sync_inside_dispatched (); + client = nmtstc_client_new (TRUE); connections = nm_client_get_connections (client); g_assert (connections); @@ -1216,7 +1103,7 @@ test_connection_invalid (void) FALSE, &path3); - nmtst_main_loop_run (loop, 1000); + nmtst_main_loop_run (gl.loop, 1000); connections = nm_client_get_connections (client); g_assert (connections); @@ -1252,7 +1139,7 @@ test_connection_invalid (void) variant, FALSE); - nmtst_main_loop_run (loop, 100); + nmtst_main_loop_run (gl.loop, 100); connections = nm_client_get_connections (client); g_assert (connections); @@ -1290,7 +1177,7 @@ test_connection_invalid (void) variant, FALSE); - nmtst_main_loop_run (loop, 100); + nmtst_main_loop_run (gl.loop, 100); connections = nm_client_get_connections (client); g_assert (connections); @@ -1325,7 +1212,7 @@ test_connection_invalid (void) connection, FALSE); - nmtst_main_loop_run (loop, 100); + nmtst_main_loop_run (gl.loop, 100); connections = nm_client_get_connections (client); g_assert (connections); @@ -1367,7 +1254,7 @@ test_connection_invalid (void) connection, FALSE); - nmtst_main_loop_run (loop, 100); + nmtst_main_loop_run (gl.loop, 100); connections = nm_client_get_connections (client); g_assert (connections); @@ -1409,7 +1296,7 @@ test_connection_invalid (void) connection, FALSE); - nmtst_main_loop_run (loop, 100); + nmtst_main_loop_run (gl.loop, 100); connections = nm_client_get_connections (client); g_assert (connections); @@ -1448,7 +1335,7 @@ main (int argc, char **argv) nmtst_init (&argc, &argv, TRUE); - loop = g_main_loop_new (NULL, FALSE); + gl.loop = g_main_loop_new (NULL, FALSE); g_test_add_func ("/libnm/device-added", test_device_added); g_test_add_func ("/libnm/device-added-signal-after-init", test_device_added_signal_after_init); diff --git a/libnm/tests/test-remote-settings-client.c b/libnm/tests/test-remote-settings-client.c index 701780897..8483eca83 100644 --- a/libnm/tests/test-remote-settings-client.c +++ b/libnm/tests/test-remote-settings-client.c @@ -8,12 +8,16 @@ #include #include +#include "nm-glib-aux/nm-time-utils.h" + #include "nm-test-libnm-utils.h" -static NMTstcServiceInfo *sinfo; -static NMClient *client = NULL; -GDBusConnection *bus = NULL; -NMRemoteConnection *remote = NULL; +static struct { + NMTstcServiceInfo *sinfo; + NMClient *client; + GDBusConnection *bus; + NMRemoteConnection *remote; +} gl = { }; /*****************************************************************************/ @@ -25,17 +29,17 @@ add_cb (GObject *s, gboolean *done = user_data; GError *error = NULL; - remote = nm_client_add_connection_finish (client, result, &error); + gl.remote = nm_client_add_connection_finish (gl.client, result, &error); g_assert_no_error (error); *done = TRUE; - g_object_add_weak_pointer (G_OBJECT (remote), (void **) &remote); + g_object_add_weak_pointer (G_OBJECT (gl.remote), (void **) &gl.remote); /* nm_client_add_connection_finish() adds a ref to @remote, but we * want the weak pointer to be cleared as soon as @client drops its own ref. * So drop ours. */ - g_object_unref (remote); + g_object_unref (gl.remote); } #define TEST_CON_ID "blahblahblah" @@ -44,32 +48,27 @@ static void test_add_connection (void) { NMConnection *connection; - time_t start, now; gboolean done = FALSE; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; connection = nmtst_create_minimal_connection (TEST_CON_ID, NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); - nm_client_add_connection_async (client, + nm_client_add_connection_async (gl.client, connection, TRUE, NULL, add_cb, &done); - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - } while ((done == FALSE) && (now - start < 5)); - g_assert (done == TRUE); - g_assert (remote != NULL); + nmtst_main_context_iterate_until (NULL, 5000, done); + + g_assert (gl.remote != NULL); /* Make sure the connection is the same as what we added */ g_assert (nm_connection_compare (connection, - NM_CONNECTION (remote), + NM_CONNECTION (gl.remote), NM_SETTING_COMPARE_FLAG_EXACT) == TRUE); g_object_unref (connection); } @@ -99,7 +98,7 @@ visible_changed_cb (GObject *object, GParamSpec *pspec, gboolean *done) static void connection_removed_cb (NMClient *s, NMRemoteConnection *connection, gboolean *done) { - if (connection == remote) + if (connection == gl.remote) *done = TRUE; } @@ -116,7 +115,6 @@ invis_has_settings_cb (NMSetting *setting, static void test_make_invisible (void) { - time_t start, now; const GPtrArray *conns; int i; GDBusProxy *proxy; @@ -124,17 +122,17 @@ test_make_invisible (void) gboolean has_settings = FALSE; char *path; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; - g_assert (remote != NULL); + g_assert (gl.remote != NULL); /* Listen for the remove event when the connection becomes invisible */ - g_signal_connect (remote, "notify::" NM_REMOTE_CONNECTION_VISIBLE, G_CALLBACK (visible_changed_cb), &visible_changed); - g_signal_connect (client, "connection-removed", G_CALLBACK (connection_removed_cb), &connection_removed); + g_signal_connect (gl.remote, "notify::" NM_REMOTE_CONNECTION_VISIBLE, G_CALLBACK (visible_changed_cb), &visible_changed); + g_signal_connect (gl.client, "connection-removed", G_CALLBACK (connection_removed_cb), &connection_removed); - path = g_strdup (nm_connection_get_path (NM_CONNECTION (remote))); - proxy = g_dbus_proxy_new_sync (bus, + path = g_strdup (nm_connection_get_path (NM_CONNECTION (gl.remote))); + proxy = g_dbus_proxy_new_sync (gl.bus, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, NULL, NM_DBUS_SERVICE, @@ -153,29 +151,23 @@ test_make_invisible (void) set_visible_cb, NULL); /* Wait for the connection to be removed */ - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - } while ((!visible_changed || !connection_removed) && (now - start < 5)); - g_assert (visible_changed == TRUE); - g_assert (connection_removed == TRUE); + nmtst_main_context_iterate_until (NULL, 5000, visible_changed && connection_removed); - g_signal_handlers_disconnect_by_func (remote, G_CALLBACK (visible_changed_cb), &visible_changed); - g_signal_handlers_disconnect_by_func (client, G_CALLBACK (connection_removed_cb), &connection_removed); + g_signal_handlers_disconnect_by_func (gl.remote, G_CALLBACK (visible_changed_cb), &visible_changed); + g_signal_handlers_disconnect_by_func (gl.client, G_CALLBACK (connection_removed_cb), &connection_removed); /* Ensure NMClient no longer has the connection */ - conns = nm_client_get_connections (client); + conns = nm_client_get_connections (gl.client); for (i = 0; i < conns->len; i++) { NMConnection *candidate = NM_CONNECTION (conns->pdata[i]); - g_assert ((gpointer) remote != (gpointer) candidate); + g_assert ((gpointer) gl.remote != (gpointer) candidate); g_assert (strcmp (path, nm_connection_get_path (candidate)) != 0); } /* And ensure the invisible connection no longer has any settings */ - g_assert (remote); - nm_connection_for_each_setting_value (NM_CONNECTION (remote), + g_assert (gl.remote); + nm_connection_for_each_setting_value (NM_CONNECTION (gl.remote), invis_has_settings_cb, &has_settings); g_assert (has_settings == FALSE); @@ -197,7 +189,6 @@ vis_new_connection_cb (NMClient *foo, static void test_make_visible (void) { - time_t start, now; const GPtrArray *conns; int i; GDBusProxy *proxy; @@ -205,17 +196,17 @@ test_make_visible (void) char *path; NMRemoteConnection *new = NULL; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; - g_assert (remote != NULL); + g_assert (gl.remote != NULL); /* Wait for the new-connection signal when the connection is visible again */ - g_signal_connect (client, NM_CLIENT_CONNECTION_ADDED, + g_signal_connect (gl.client, NM_CLIENT_CONNECTION_ADDED, G_CALLBACK (vis_new_connection_cb), &new); - path = g_strdup (nm_connection_get_path (NM_CONNECTION (remote))); - proxy = g_dbus_proxy_new_sync (bus, + path = g_strdup (nm_connection_get_path (NM_CONNECTION (gl.remote))); + proxy = g_dbus_proxy_new_sync (gl.bus, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, NULL, NM_DBUS_SERVICE, @@ -234,24 +225,19 @@ test_make_visible (void) set_visible_cb, NULL); /* Wait for the settings service to announce the connection again */ - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - } while ((new == NULL) && (now - start < 5)); + nmtst_main_context_iterate_until (NULL, 5000, new); /* Ensure the new connection is the same as the one we made visible again */ - g_assert (new); - g_assert (new == remote); + g_assert (new == gl.remote); - g_signal_handlers_disconnect_by_func (client, G_CALLBACK (vis_new_connection_cb), &new); + g_signal_handlers_disconnect_by_func (gl.client, G_CALLBACK (vis_new_connection_cb), &new); /* Ensure NMClient has the connection */ - conns = nm_client_get_connections (client); + conns = nm_client_get_connections (gl.client); for (i = 0; i < conns->len; i++) { NMConnection *candidate = NM_CONNECTION (conns->pdata[i]); - if ((gpointer) remote == (gpointer) candidate) { + if ((gpointer) gl.remote == (gpointer) candidate) { g_assert_cmpstr (path, ==, nm_connection_get_path (candidate)); g_assert_cmpstr (TEST_CON_ID, ==, nm_connection_get_id (candidate)); found = TRUE; @@ -282,7 +268,7 @@ deleted_cb (GObject *proxy, static void removed_cb (NMClient *s, NMRemoteConnection *connection, gboolean *done) { - if (connection == remote) + if (connection == gl.remote) *done = TRUE; } @@ -290,27 +276,26 @@ static void test_remove_connection (void) { NMRemoteConnection *connection; - time_t start, now; const GPtrArray *conns; int i; GDBusProxy *proxy; gboolean done = FALSE; char *path; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; /* Find a connection to delete */ - conns = nm_client_get_connections (client); + conns = nm_client_get_connections (gl.client); g_assert_cmpint (conns->len, >, 0); connection = NM_REMOTE_CONNECTION (conns->pdata[0]); g_assert (connection); - g_assert (remote == connection); + g_assert (gl.remote == connection); path = g_strdup (nm_connection_get_path (NM_CONNECTION (connection))); - g_signal_connect (client, "connection-removed", G_CALLBACK (removed_cb), &done); + g_signal_connect (gl.client, "connection-removed", G_CALLBACK (removed_cb), &done); - proxy = g_dbus_proxy_new_sync (bus, + proxy = g_dbus_proxy_new_sync (gl.bus, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, NULL, NM_DBUS_SERVICE, @@ -328,18 +313,10 @@ test_remove_connection (void) NULL, deleted_cb, NULL); - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - if (done && !remote) - break; - } while (now - start < 5); - g_assert (done == TRUE); - g_assert (!remote); + nmtst_main_context_iterate_until (NULL, 5000, done && !gl.remote); /* Ensure NMClient no longer has the connection */ - conns = nm_client_get_connections (client); + conns = nm_client_get_connections (gl.client); for (i = 0; i < conns->len; i++) { NMConnection *candidate = NM_CONNECTION (conns->pdata[i]); @@ -364,7 +341,7 @@ add_remove_cb (GObject *s, gboolean *done = user_data; gs_free_error GError *error = NULL; - connection = nm_client_add_connection_finish (client, result, &error); + connection = nm_client_add_connection_finish (gl.client, result, &error); g_assert_error (error, NM_CLIENT_ERROR, NM_CLIENT_ERROR_OBJECT_CREATION_FAILED); g_assert (connection == NULL); @@ -374,43 +351,34 @@ add_remove_cb (GObject *s, static void test_add_remove_connection (void) { - GVariant *ret; + gs_unref_variant GVariant *ret = NULL; GError *error = NULL; - NMConnection *connection; - time_t start, now; + gs_unref_object NMConnection *connection = NULL; gboolean done = FALSE; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; /* This will cause the test server to immediately delete the connection * after creating it. */ - ret = g_dbus_proxy_call_sync (sinfo->proxy, + ret = g_dbus_proxy_call_sync (gl.sinfo->proxy, "AutoRemoveNextConnection", NULL, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error); - g_assert_no_error (error); - g_variant_unref (ret); + nmtst_assert_success (ret, error); connection = nmtst_create_minimal_connection (TEST_ADD_REMOVE_ID, NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); - nm_client_add_connection_async (client, + nm_client_add_connection_async (gl.client, connection, TRUE, NULL, add_remove_cb, &done); - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - } while ((done == FALSE) && (now - start < 5)); - g_assert (done == TRUE); - - g_object_unref (connection); + nmtst_main_context_iterate_until (NULL, 5000, done); } /*****************************************************************************/ @@ -423,7 +391,7 @@ add_bad_cb (GObject *s, gboolean *done = user_data; gs_free_error GError *error = NULL; - remote = nm_client_add_connection_finish (client, result, &error); + gl.remote = nm_client_add_connection_finish (gl.client, result, &error); g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY); *done = TRUE; @@ -432,31 +400,25 @@ add_bad_cb (GObject *s, static void test_add_bad_connection (void) { - NMConnection *connection; - time_t start, now; + gs_unref_object NMConnection *connection = NULL; gboolean done = FALSE; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; /* The test daemon doesn't support bond connections */ connection = nmtst_create_minimal_connection ("bad connection test", NULL, NM_SETTING_BOND_SETTING_NAME, NULL); - nm_client_add_connection_async (client, + nm_client_add_connection_async (gl.client, connection, TRUE, NULL, add_bad_cb, &done); - g_object_unref (connection); + g_clear_object (&connection); - start = time (NULL); - do { - now = time (NULL); - g_main_context_iteration (NULL, FALSE); - } while ((done == FALSE) && (now - start < 5)); - g_assert (done == TRUE); - g_assert (remote == NULL); + nmtst_main_context_iterate_until (NULL, 5000, done); + g_assert (gl.remote == NULL); } /*****************************************************************************/ @@ -469,7 +431,7 @@ save_hostname_cb (GObject *s, gboolean *done = user_data; gs_free_error GError *error = NULL; - nm_client_save_hostname_finish (client, result, &error); + nm_client_save_hostname_finish (gl.client, result, &error); g_assert_no_error (error); *done = TRUE; @@ -478,27 +440,30 @@ save_hostname_cb (GObject *s, static void test_save_hostname (void) { - time_t start, now; + gint64 until_ts; gboolean done = FALSE; GError *error = NULL; - if (!nmtstc_service_available (sinfo)) + if (!nmtstc_service_available (gl.sinfo)) return; /* test-networkmanager-service.py requires the hostname to contain a '.' */ - nm_client_save_hostname (client, "foo", NULL, &error); + nm_client_save_hostname (gl.client, "foo", NULL, &error); g_assert_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_HOSTNAME); g_clear_error (&error); - nm_client_save_hostname_async (client, "example.com", NULL, save_hostname_cb, &done); + nm_client_save_hostname_async (gl.client, "example.com", NULL, save_hostname_cb, &done); - start = time (NULL); - do { - now = time (NULL); + until_ts = nm_utils_get_monotonic_timestamp_ms () + 5000; + while (TRUE) { g_main_context_iteration (NULL, FALSE); - } while ((done == FALSE) && (now - start < 5)); - g_assert (done == TRUE); - g_assert (remote == NULL); + if (done) + break; + if (nm_utils_get_monotonic_timestamp_ms () >= until_ts) + g_assert_not_reached (); + } + + g_assert (gl.remote == NULL); } /*****************************************************************************/ @@ -515,14 +480,12 @@ main (int argc, char **argv) nmtst_init (&argc, &argv, TRUE); - bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); - g_assert_no_error (error); + gl.bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); + nmtst_assert_success (gl.bus, error); - sinfo = nmtstc_service_init (); + gl.sinfo = nmtstc_service_init (); - client = nm_client_new (NULL, &error); - g_assert_no_error (error); - g_assert (client != NULL); + gl.client = nmtstc_client_new (TRUE); /* FIXME: these tests assume that they get run in order, but g_test_run() * does not actually guarantee that! @@ -537,9 +500,9 @@ main (int argc, char **argv) ret = g_test_run (); - nmtstc_service_cleanup (sinfo); - g_object_unref (client); - g_object_unref (bus); + nm_clear_pointer (&gl.sinfo, nmtstc_service_cleanup); + g_clear_object (&gl.client); + g_clear_object (&gl.bus); return ret; } diff --git a/libnm/tests/test-secret-agent.c b/libnm/tests/test-secret-agent.c index bb822a0a0..fef6071df 100644 --- a/libnm/tests/test-secret-agent.c +++ b/libnm/tests/test-secret-agent.c @@ -234,8 +234,9 @@ test_setup (TestSecretAgentData *sadata, gconstpointer test_data) if (!sadata->sinfo) return; - sadata->client = nm_client_new (NULL, &error); - g_assert_no_error (error); + g_assert (g_main_context_get_thread_default () == NULL); + + sadata->client = nmtstc_client_new (TRUE); sadata->loop = g_main_loop_new (NULL, FALSE); sadata->timeout_id = g_timeout_add_seconds (5, timeout_assert, NULL); From feea4222eff1be4576b959f168fd800d0cf3e317 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 Nov 2019 21:41:03 +0100 Subject: [PATCH 17/22] libnm/tests: unsubscribe signal handler during test_activate_virtual() libnm is gonna change, where it would still emit signals when the instance gets destructed. In particular, when the device gets removed from the NMClient cache, the references to other objects would be cleared (and consequently property changed signals emitted). This will cause a test failure, because the signal was not unsubscribed: test:ERROR:libnm/tests/test-nm-client.c:694:device_ac_changed_cb: assertion failed: (nm_device_get_active_connection (NM_DEVICE (device)) != NULL) --- libnm/tests/test-nm-client.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index 987b63ed9..e92e1cd1e 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -606,6 +606,9 @@ typedef struct { NMActiveConnection *ac; int remaining; + + NMDevice *device; + gulong ac_signal_id; } TestACInfo; static void @@ -768,9 +771,12 @@ client_devices_changed_cb (GObject *client, g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0.1"); if (!nm_device_get_active_connection (device)) { + g_assert (info->ac_signal_id == 0); info->remaining++; - g_signal_connect (device, "notify::" NM_DEVICE_ACTIVE_CONNECTION, - G_CALLBACK (device_ac_changed_cb), info); + info->device = device; + g_object_add_weak_pointer (G_OBJECT (device), (gpointer *) &info->device); + info->ac_signal_id = g_signal_connect (device, "notify::" NM_DEVICE_ACTIVE_CONNECTION, + G_CALLBACK (device_ac_changed_cb), info); } info->remaining--; @@ -872,8 +878,12 @@ test_activate_virtual (void) g_signal_handlers_disconnect_by_func (client, client_devices_changed_cb, &info); g_assert (info.ac != NULL); + g_clear_object (&info.ac); - g_object_unref (info.ac); + if (info.device) { + g_object_remove_weak_pointer (G_OBJECT (info.device), (gpointer *) &info.device); + nm_clear_g_signal_handler (info.device, &info.ac_signal_id); + } } static void From 6bf206eb810f3f72115e2194f615fce88fb3b0f2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 Nov 2019 22:26:04 +0100 Subject: [PATCH 18/22] libnm/tests: drop test_activate_failed() test With this test the stub service simulates a failure to add-and-activate the connection. However the implementation of the stub service was not simulating the real behavior of NetworkManager service. libnm will add the possibility to assert against invalid server behavior by setting LIBNM_CLIENT_DEBUG=error. With that change, libnm will complain that the stub service behaves invalid, and rightly so. Instead of fixing the test, just drop it. --- libnm/tests/test-nm-client.c | 43 ---------------------------- tools/test-networkmanager-service.py | 8 ------ 2 files changed, 51 deletions(-) diff --git a/libnm/tests/test-nm-client.c b/libnm/tests/test-nm-client.c index e92e1cd1e..c6b26a21a 100644 --- a/libnm/tests/test-nm-client.c +++ b/libnm/tests/test-nm-client.c @@ -886,48 +886,6 @@ test_activate_virtual (void) } } -static void -activate_failed_cb (GObject *object, - GAsyncResult *result, - gpointer user_data) -{ - NMClient *client = NM_CLIENT (object); - NMActiveConnection *ac; - GError *error = NULL; - - ac = nm_client_add_and_activate_connection_finish (client, result, &error); - g_assert (ac == NULL); - g_assert_error (error, NM_CLIENT_ERROR, NM_CLIENT_ERROR_OBJECT_CREATION_FAILED); - g_clear_error (&error); - - g_main_loop_quit (gl.loop); -} - -static void -test_activate_failed (void) -{ - nmtstc_auto_service_cleanup NMTstcServiceInfo *sinfo = NULL; - gs_unref_object NMClient *client = NULL; - NMDevice *device; - gs_unref_object NMConnection *conn = NULL; - - sinfo = nmtstc_service_init (); - if (!nmtstc_service_available (sinfo)) - return; - - client = nmtstc_client_new (TRUE); - - device = nmtstc_service_add_device (sinfo, client, "AddWiredDevice", "eth0"); - - /* Note that test-networkmanager-service.py checks for this exact name */ - conn = nmtst_create_minimal_connection ("object-creation-failed-test", NULL, - NM_SETTING_WIRED_SETTING_NAME, NULL); - - nm_client_add_and_activate_connection_async (client, conn, device, NULL, - NULL, activate_failed_cb, NULL); - g_main_loop_run (gl.loop); -} - static void test_device_connection_compatibility (void) { @@ -1354,7 +1312,6 @@ main (int argc, char **argv) g_test_add_func ("/libnm/client-nm-running", test_client_nm_running); g_test_add_func ("/libnm/active-connections", test_active_connections); g_test_add_func ("/libnm/activate-virtual", test_activate_virtual); - g_test_add_func ("/libnm/activate-failed", test_activate_failed); g_test_add_func ("/libnm/device-connection-compatibility", test_device_connection_compatibility); g_test_add_func ("/libnm/connection/invalid", test_connection_invalid); diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index e91b45233..1a8e2e6af 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -1306,14 +1306,6 @@ class NetworkManager(ExportedObj): ac = ActiveConnection(device, con_inst, None) self.active_connection_add(ac) - - if NmUtil.con_hash_get_id(con_hash) == 'object-creation-failed-test': - # FIXME: this is not the right test, to delete the active-connection - # before returning it. It's the wrong order of what NetworkManager - # would do. - self.active_connection_remove(ac) - return ExportedObj.to_path(ac) - return ExportedObj.to_path(ac) def active_connection_add(self, ac): From 44a56fca606d1a9171fd3e9ba55882d42e747695 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 Nov 2019 23:06:31 +0100 Subject: [PATCH 19/22] libnm/tests: fix stub implementation for remote-next-connection We cannot first remove the connection (and emit property changed signals), before replying with the newly added path (that already no longer exists). Fix the stub implementation. --- tools/test-networkmanager-service.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index 1a8e2e6af..03ed7bde5 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -1592,6 +1592,9 @@ class Connection(ExportedObj): @dbus.service.method(dbus_interface=IFACE_CONNECTION, in_signature='', out_signature='a{sa{sv}}') def GetSettings(self): + if hasattr(self, '_remove_next_connection_cb'): + self._remove_next_connection_cb() + raise BusErr.UnknownConnectionException("Connection not found") if not self.visible: raise BusErr.PermissionDeniedException() return self.con_hash @@ -1694,11 +1697,21 @@ class Settings(ExportedObj): self.NewConnection(con_inst.path) self._dbus_property_set(IFACE_SETTINGS, PRP_SETTINGS_CONNECTIONS, dbus.Array(self.get_connection_paths(), 'o')) + gl.manager.devices_available_connections_update() + if self.remove_next_connection: self.remove_next_connection = False - self.delete_connection(con_inst) - - gl.manager.devices_available_connections_update() + def cb(): + if hasattr(con_inst, '_remove_next_connection_cb'): + del con_inst._remove_next_connection_cb + self.delete_connection(con_inst) + return False + # We will delete the connection right away on an idle handler. However, + # the test races with initializing the connection (calling GetSettings()). + # To avoid the race, we will check in GetSettings() whether the profile + # is about to be deleted, and delete it first. + con_inst._remove_next_connection_cb = cb + GLib.idle_add(cb) return con_inst.path From 6a9ed0adfb896efd341531cfed5c8c466048546b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 1 Nov 2019 12:59:03 +0100 Subject: [PATCH 20/22] cli: unsubscribe permission signal from NMClient on exit During the libnm rework, we might still emit permissions changed signal while destructing the instance. That triggers an assertion. Backtrace, with a different libnm: #0 _g_log_abort (breakpoint=1) at ../glib/gmessages.c:554 #1 0x00007ffff77d09b6 in g_logv (log_domain=0x7ffff7f511cd "libnm", log_level=G_LOG_LEVEL_CRITICAL, format=, args=args@entry=0x7fffffffcb80) at ../glib/gmessages.c:1373 #2 0x00007ffff77d0b83 in g_log (log_domain=log_domain@entry=0x7ffff7f511cd "libnm", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff78215df "%s: assertion '%s' failed") at ../glib/gmessages.c:1415 #3 0x00007ffff77d137d in g_return_if_fail_warning (log_domain=log_domain@entry=0x7ffff7f511cd "libnm", pretty_function=pretty_function@entry=0x7ffff7f58aa0 <__func__.40223> "nm_client_get_permission_result", expression=expression@entry=0x7ffff7f54830 "NM_IS_CLIENT (client)") at ../glib/gmessages.c:2771 #4 0x00007ffff7e9de9a in nm_client_get_permission_result (client=0x0, permission=permission@entry=NM_CLIENT_PERMISSION_ENABLE_DISABLE_NETWORK) at libnm/nm-client.c:3816 #5 0x0000555555593ba3 in got_permissions (nmc=nmc@entry=0x55555562ec20 ) at clients/cli/general.c:587 #6 0x0000555555593bcb in permission_changed (client=, permission=, result=, nmc=0x55555562ec20 ) at clients/cli/general.c:600 #7 0x00007ffff73b1aa8 in ffi_call_unix64 () at ../src/x86/unix64.S:76 #8 0x00007ffff73b12a4 in ffi_call (cif=cif@entry=0x7fffffffced0, fn=fn@entry=0x555555593bbf , rvalue=, avalue=avalue@entry=0x7fffffffcde0) at ../src/x86/ffi64.c:525 #9 0x00007ffff78b4746 in g_cclosure_marshal_generic_va (closure=, return_value=, instance=, args_list=, marshal_data=, n_params=, param_types=) at ../gobject/gclosure.c:1614 #10 0x00007ffff78b3996 in _g_closure_invoke_va (closure=0x5555556f4330, return_value=0x0, instance=0x55555565a020, args=0x7fffffffd180, n_params=2, param_types=0x555555656f00) at ../gobject/gclosure.c:873 #11 0x00007ffff78d0228 in g_signal_emit_valist (instance=0x55555565a020, signal_id=, detail=0, var_args=var_args@entry=0x7fffffffd180) at ../gobject/gsignal.c:3306 #12 0x00007ffff78d09d3 in g_signal_emit (instance=instance@entry=0x55555565a020, signal_id=, detail=detail@entry=0) at ../gobject/gsignal.c:3453 #13 0x00007ffff7e8989a in _emit_permissions_changed (self=self@entry=0x55555565a020, permissions=permissions@entry=0x555555690e40 = {...}, force_unknown=force_unknown@entry=1) at libnm/nm-client.c:2874 #14 0x00007ffff7e9a0c9 in _init_release_all (self=self@entry=0x55555565a020) at libnm/nm-client.c:6092 #15 0x00007ffff7e9bcde in dispose (object=0x55555565a020 [NMClient]) at libnm/nm-client.c:6838 #16 0x00007ffff78b8c28 in g_object_unref (_object=) at ../gobject/gobject.c:3344 #17 g_object_unref (_object=0x55555565a020) at ../gobject/gobject.c:3274 #18 0x00005555555badcf in nmc_cleanup (nmc=0x55555562ec20 ) at clients/cli/nmcli.c:924 #19 0x00005555555bbea7 in main (argc=, argv=0x7fffffffd498) at clients/cli/nmcli.c:987 --- clients/cli/general.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clients/cli/general.c b/clients/cli/general.c index e246d2828..123cc5b4f 100644 --- a/clients/cli/general.c +++ b/clients/cli/general.c @@ -600,6 +600,9 @@ permission_changed (NMClient *client, if (got_permissions (nmc)) { /* Defer the printing, so that we have a chance to process the other * permission-changed signals. */ + g_signal_handlers_disconnect_by_func (nmc->client, + G_CALLBACK (permission_changed), + nmc); g_idle_remove_by_data (nmc); g_idle_add (print_permissions, nmc); } @@ -616,7 +619,7 @@ show_nm_permissions (NmCli *nmc) /* The client didn't get the permissions reply yet. Subscribe to changes. */ g_signal_connect (nmc->client, NM_CLIENT_PERMISSION_CHANGED, - G_CALLBACK (permission_changed), nmc); + G_CALLBACK (permission_changed), nmc); if (nmc->timeout == -1) nmc->timeout = 10; From cdb95062e79ef46dd8407591543bf6c73af65459 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Nov 2019 13:52:25 +0100 Subject: [PATCH 21/22] clients/tests: set NM_TEST_CALLING_NUM environement variable for client tests Debugging tests that are called by test-client.py is cumbersome. One way would be to set NM_TEST_CLIENT_NMCLI_PATH to a wrapper script. However, then we want to know from the wrapper script which test we are currently calling. Add that to the environment. --- clients/tests/test-client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py index af26756de..5ca150eb5 100755 --- a/clients/tests/test-client.py +++ b/clients/tests/test-client.py @@ -670,6 +670,7 @@ class TestNmcli(NmTestBase): env['TERM'] = 'linux' env['ASAN_OPTIONS'] = 'detect_leaks=0' env['XDG_CONFIG_HOME'] = PathConfiguration.srcdir() + env['NM_TEST_CALLING_NUM'] = str(calling_num) if fatal_warnings is _DEFAULT_ARG or fatal_warnings: env['G_DEBUG'] = 'fatal-warnings' From e96bc190d60a574f6a02abe2b9efdcf44558310f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Nov 2019 15:30:55 +0100 Subject: [PATCH 22/22] clients/tests: read stdout/stderr buffers during test-clients.py We need to actually read the stdout/stderr of the nmcli programs. Otherwise, the pipe might fill uup and block to process (eventually leading to timeout). --- clients/tests/test-client.py | 65 +++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py index 5ca150eb5..10739239b 100755 --- a/clients/tests/test-client.py +++ b/clients/tests/test-client.py @@ -91,11 +91,13 @@ import itertools import subprocess import shlex import re +import fcntl import dbus import time import random import dbus.service import dbus.mainloop.glib +import io ############################################################################### @@ -173,26 +175,52 @@ class Util: @staticmethod def popen_wait(p, timeout = 0): - if Util.python_has_version(3, 3): - if timeout == 0: - return p.poll() - try: - return p.wait(timeout) - except subprocess.TimeoutExpired: - return None + (res, b_stdout, b_stderr) = Util.popen_wait_read(p, timeout = timeout, read_std_pipes = False) + return res + + @staticmethod + def popen_wait_read(p, timeout = 0, read_std_pipes = True): start = NM.utils_get_timestamp_msec() delay = 0.0005 + b_stdout = b'' + b_stderr = b'' + res = None while True: + if read_std_pipes: + b_stdout += Util.buffer_read(p.stdout) + b_stderr += Util.buffer_read(p.stderr) if p.poll() is not None: - return p.returncode + res = p.returncode + break if timeout == 0: - return None + break assert(timeout > 0) remaining = timeout - ((NM.utils_get_timestamp_msec() - start) / 1000.0) if remaining <= 0: - return None + break delay = min(delay * 2, remaining, 0.05) time.sleep(delay) + return (res, b_stdout, b_stderr) + + @staticmethod + def buffer_read(buf): + b = b'' + while True: + try: + b1 = buf.read() + except io.BlockingIOError: + b1 = b'' + except IOError: + b1 = b'' + if not b1: + return b + b += b1 + + @staticmethod + def buffer_set_nonblock(buf): + fd = buf.fileno() + fl = fcntl.fcntl(fd, fcntl.F_GETFL) + fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK) @staticmethod def random_job(jobs): @@ -459,10 +487,14 @@ class AsyncProcess(): def start(self): if not hasattr(self, '_p'): self._p_start_timestamp = NM.utils_get_timestamp_msec() + self._p_stdout_buf = b'' + self._p_stderr_buf = b'' self._p = subprocess.Popen(self._args, stdout = subprocess.PIPE, stderr = subprocess.PIPE, env = self._env) + Util.buffer_set_nonblock(self._p.stdout) + Util.buffer_set_nonblock(self._p.stderr) def _timeout_remaining_time(self): # note that we call this during poll() and wait_and_complete(). @@ -476,7 +508,11 @@ class AsyncProcess(): def poll(self, timeout = 0): self.start() - return_code = Util.popen_wait(self._p, timeout) + (return_code, b_stdout, b_stderr) = Util.popen_wait_read(self._p, timeout) + + self._p_stdout_buf += b_stdout + self._p_stderr_buf += b_stderr + if return_code is None \ and self._timeout_remaining_time() <= 0: raise Exception("process is still running after timeout: %s" % (' '.join(self._args))) @@ -488,11 +524,16 @@ class AsyncProcess(): p = self._p self._p = None - return_code = Util.popen_wait(p, max(0, self._timeout_remaining_time()) / 1000) + (return_code, b_stdout, b_stderr) = Util.popen_wait_read(p, max(0, self._timeout_remaining_time()) / 1000) (stdout, stderr) = (p.stdout.read(), p.stderr.read()) p.stdout.close() p.stderr.close() + stdout = self._p_stdout_buf + b_stdout + stdout + stderr = self._p_stderr_buf + b_stderr + stderr + del self._p_stdout_buf + del self._p_stderr_buf + if return_code is None: print(stdout) print(stderr)