From 194443237a8715d45e794f248b82ef9e391a0a76 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Oct 2019 13:20:54 +0200 Subject: [PATCH 01/47] libnm/trivial: remove unused struct RequestScanInfo in "nm-device-wifi-p2p.c" --- libnm/nm-device-wifi-p2p.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libnm/nm-device-wifi-p2p.c b/libnm/nm-device-wifi-p2p.c index e107721190..bbb2211824 100644 --- a/libnm/nm-device-wifi-p2p.c +++ b/libnm/nm-device-wifi-p2p.c @@ -19,11 +19,6 @@ /*****************************************************************************/ -typedef struct { - NMDeviceWifiP2P *device; - GSimpleAsyncResult *simple; -} RequestScanInfo; - NM_GOBJECT_PROPERTIES_DEFINE_BASE ( PROP_HW_ADDRESS, PROP_PEERS, From d80af0225e90528dd4cf9d01cf91dc22d15aaec8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 4 Oct 2019 16:48:39 +0200 Subject: [PATCH 02/47] libnm: mark more synchronous libnm API as deprecated This is a follow-up to commit e90684a169a7 ('libnm: deprecate synchronous/blocking API in libnm') to mark more of such synchronous API as deprecated. --- libnm/nm-client.c | 8 ++++++++ libnm/nm-client.h | 3 +++ libnm/nm-manager.h | 3 +++ 3 files changed, 14 insertions(+) diff --git a/libnm/nm-client.c b/libnm/nm-client.c index fd6c35dcae..d5ae5d4ac3 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -577,6 +577,8 @@ nm_client_connectivity_check_get_uri (NMClient *client) * Gets NetworkManager current logging level and domains. * * Returns: %TRUE on success, %FALSE otherwise + * + * Deprecated: 1.22, use nm_client_get_logging_async() or GDBusConnection **/ gboolean nm_client_get_logging (NMClient *client, char **level, char **domains, GError **error) @@ -586,6 +588,8 @@ nm_client_get_logging (NMClient *client, char **level, char **domains, GError ** g_return_val_if_fail (domains == NULL || *domains == NULL, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + /* FIXME(libnm-async-api): add nm_client_get_logging_async(). */ + if (!_nm_client_check_nm_running (client, error)) return FALSE; @@ -604,6 +608,8 @@ nm_client_get_logging (NMClient *client, char **level, char **domains, GError ** * Sets NetworkManager logging level and/or domains. * * Returns: %TRUE on success, %FALSE otherwise + * + * Deprecated: 1.22, use nm_client_set_logging_async() or GDBusConnection **/ gboolean nm_client_set_logging (NMClient *client, const char *level, const char *domains, GError **error) @@ -611,6 +617,8 @@ nm_client_set_logging (NMClient *client, const char *level, const char *domains, g_return_val_if_fail (NM_IS_CLIENT (client), FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + /* FIXME(libnm-async-api): add nm_client_set_logging_async(). */ + if (!_nm_client_check_nm_running (client, error)) return FALSE; diff --git a/libnm/nm-client.h b/libnm/nm-client.h index 0b3b0e36e7..32cbd168a0 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -277,10 +277,13 @@ void nm_client_connectivity_check_set_enabled (NMClient *client, NM_AVAILABLE_IN_1_20 const char *nm_client_connectivity_check_get_uri (NMClient *client); +_NM_DEPRECATED_SYNC_METHOD gboolean nm_client_get_logging (NMClient *client, char **level, char **domains, GError **error); + +_NM_DEPRECATED_SYNC_METHOD gboolean nm_client_set_logging (NMClient *client, const char *level, const char *domains, diff --git a/libnm/nm-manager.h b/libnm/nm-manager.h index 9b46b6417d..e9c09c62c5 100644 --- a/libnm/nm-manager.h +++ b/libnm/nm-manager.h @@ -112,10 +112,13 @@ void nm_manager_connectivity_check_set_enabled (NMManager *manager, const char *nm_manager_connectivity_check_get_uri (NMManager *manager); +_NM_DEPRECATED_SYNC_METHOD_INTERNAL gboolean nm_manager_get_logging (NMManager *manager, char **level, char **domains, GError **error); + +_NM_DEPRECATED_SYNC_METHOD_INTERNAL gboolean nm_manager_set_logging (NMManager *manager, const char *level, const char *domains, From 75a04a8a54832715597216117b2dedb7fb2403e7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 21:05:36 +0200 Subject: [PATCH 03/47] libnm: fix annotation for return value of nm_remote_connection_update2() --- libnm/nm-remote-connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm/nm-remote-connection.c b/libnm/nm-remote-connection.c index 04fbd3b170..4b5738e0f3 100644 --- a/libnm/nm-remote-connection.c +++ b/libnm/nm-remote-connection.c @@ -148,7 +148,7 @@ nm_remote_connection_update2 (NMRemoteConnection *connection, * * Gets the result of a call to nm_remote_connection_commit_changes_async(). * - * Returns: on success, a #GVariant of type "a{sv}" with the result. On failure, + * Returns: (transfer full): on success, a #GVariant of type "a{sv}" with the result. On failure, * %NULL. **/ GVariant * From b9ff785744ff24b23421a784d99fdee8b2be2fb8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 22:10:09 +0200 Subject: [PATCH 04/47] libnm: fix annotation for return value of nm_remote_connection_get_secrets() --- libnm/nm-remote-connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm/nm-remote-connection.c b/libnm/nm-remote-connection.c index 4b5738e0f3..6ba139e7e9 100644 --- a/libnm/nm-remote-connection.c +++ b/libnm/nm-remote-connection.c @@ -527,7 +527,7 @@ nm_remote_connection_delete_finish (NMRemoteConnection *connection, * Request the connection's secrets. Note that this is a blocking D-Bus call, * not a simple property accessor. * - * Returns: a #GVariant of type %NM_VARIANT_TYPE_CONNECTION containing + * Returns: (transfer full): a #GVariant of type %NM_VARIANT_TYPE_CONNECTION containing * @connection's secrets, or %NULL on error. * * Deprecated: 1.22, use nm_remote_connection_get_secrets_async() or GDBusConnection From 92285cfd3e48cb2889d40dd27336fa9e727b3ff8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 14:33:11 +0200 Subject: [PATCH 05/47] libnm/device: fix memleak in nm_device_wifi_request_scan_options*() Fixes: 7691fe575377 ('libnm: add new functions allowing passing options to RequestScan() D-Bus call') --- libnm/nm-device-wifi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm/nm-device-wifi.c b/libnm/nm-device-wifi.c index 30dbd09f89..5ea7eca194 100644 --- a/libnm/nm-device-wifi.c +++ b/libnm/nm-device-wifi.c @@ -288,7 +288,7 @@ prepare_scan_options (GVariant *options) else { g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); g_variant_iter_init (&iter, options); - while (g_variant_iter_loop (&iter, "{sv}", &key, &value)) + while (g_variant_iter_loop (&iter, "{&sv}", &key, &value)) { // FIXME: verify options here? g_variant_builder_add (&builder, "{sv}", key, value); From 40911fb99b893ae26577cb808b380b20df3f3117 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 14:33:11 +0200 Subject: [PATCH 06/47] libnm/device: fix memleak options variant in nm_device_wifi_request_scan_options*() A function that accepts a floating variant must consume it. Fixes: 7691fe575377 ('libnm: add new functions allowing passing options to RequestScan() D-Bus call') --- libnm/nm-device-wifi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libnm/nm-device-wifi.c b/libnm/nm-device-wifi.c index 5ea7eca194..0cdcffdfc2 100644 --- a/libnm/nm-device-wifi.c +++ b/libnm/nm-device-wifi.c @@ -294,6 +294,7 @@ prepare_scan_options (GVariant *options) g_variant_builder_add (&builder, "{sv}", key, value); } variant = g_variant_builder_end (&builder); + nm_g_variant_unref_floating (options); } return variant; } @@ -309,7 +310,7 @@ _device_wifi_request_scan (NMDeviceWifi *device, g_return_val_if_fail (NM_IS_DEVICE_WIFI (device), FALSE); - variant = prepare_scan_options (options); + variant = prepare_scan_options (g_steal_pointer (&options)); ret = nmdbus_device_wifi_call_request_scan_sync (NM_DEVICE_WIFI_GET_PRIVATE (device)->proxy, variant, @@ -432,7 +433,7 @@ _device_wifi_request_scan_async (NMDeviceWifi *device, info->device = device; info->simple = simple; - variant = prepare_scan_options (options); + variant = prepare_scan_options (g_steal_pointer (&options)); priv->scan_info = info; nmdbus_device_wifi_call_request_scan (NM_DEVICE_WIFI_GET_PRIVATE (device)->proxy, From 28d69b56429fff02153d77185ad7ffc767ed4de5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 14:41:07 +0200 Subject: [PATCH 07/47] libnm/device: simplify prepare_scan_options() for Wi-Fi scanning It doesn't actually do anything, as the FIXME comment indicates. I don't think there is anything to do either. Just simplify the function. --- libnm/nm-device-wifi.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/libnm/nm-device-wifi.c b/libnm/nm-device-wifi.c index 0cdcffdfc2..ab25370ba8 100644 --- a/libnm/nm-device-wifi.c +++ b/libnm/nm-device-wifi.c @@ -276,27 +276,8 @@ nm_device_wifi_get_last_scan (NMDeviceWifi *device) static GVariant * prepare_scan_options (GVariant *options) { - - GVariant *variant; - GVariantIter iter; - GVariantBuilder builder; - char *key; - GVariant *value; - - if (!options) - variant = g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0); - else { - g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); - g_variant_iter_init (&iter, options); - while (g_variant_iter_loop (&iter, "{&sv}", &key, &value)) - { - // FIXME: verify options here? - g_variant_builder_add (&builder, "{sv}", key, value); - } - variant = g_variant_builder_end (&builder); - nm_g_variant_unref_floating (options); - } - return variant; + return options + ?: g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0); } static gboolean @@ -306,14 +287,12 @@ _device_wifi_request_scan (NMDeviceWifi *device, GError **error) { gboolean ret; - GVariant *variant; g_return_val_if_fail (NM_IS_DEVICE_WIFI (device), FALSE); - variant = prepare_scan_options (g_steal_pointer (&options)); ret = nmdbus_device_wifi_call_request_scan_sync (NM_DEVICE_WIFI_GET_PRIVATE (device)->proxy, - variant, + prepare_scan_options (g_steal_pointer (&options)), cancellable, error); if (error && *error) g_dbus_error_strip_remote_error (*error); @@ -412,7 +391,6 @@ _device_wifi_request_scan_async (NMDeviceWifi *device, NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (device); RequestScanInfo *info; GSimpleAsyncResult *simple; - GVariant *variant; g_return_if_fail (NM_IS_DEVICE_WIFI (device)); @@ -433,11 +411,9 @@ _device_wifi_request_scan_async (NMDeviceWifi *device, info->device = device; info->simple = simple; - variant = prepare_scan_options (g_steal_pointer (&options)); - priv->scan_info = info; nmdbus_device_wifi_call_request_scan (NM_DEVICE_WIFI_GET_PRIVATE (device)->proxy, - variant, + prepare_scan_options (g_steal_pointer (&options)), cancellable, request_scan_cb, info); } From 9dac3076f741acbabee2e28c6740cb72a564a178 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 23:19:32 +0200 Subject: [PATCH 08/47] libnm: fix return value for nm_remote_settings_load_connections() to ignore server result nm_remote_settings_load_connections() and nm_remote_settings_load_connections_async() behave inconsistently. It's unexpected, that a FALSE return value may leave @error unset. Note that before commit 22e830f0469a ('settings/d-bus: fix boolean return value of "LoadConnections"'), the server boolean response would have been bogus anyway (at least for some versions). Unify the behavior, and ignore the boolean return value. --- ...rg.freedesktop.NetworkManager.Settings.xml | 1 + libnm/nm-client.c | 24 ++++++++++++------- libnm/nm-remote-settings.c | 5 ++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/introspection/org.freedesktop.NetworkManager.Settings.xml b/introspection/org.freedesktop.NetworkManager.Settings.xml index f7be2717b8..59e0d57549 100644 --- a/introspection/org.freedesktop.NetworkManager.Settings.xml +++ b/introspection/org.freedesktop.NetworkManager.Settings.xml @@ -102,6 +102,7 @@ LoadConnections: @filenames: Array of paths to on-disk connection profiles in directories monitored by NetworkManager. @status: Success or failure of the operation as a whole. True if NetworkManager at least tried to load the indicated connections, even if it did not succeed. False if an error occurred before trying to load the connections (eg, permission denied). + Note that before 1.20, NetworkManager had a bug and this @status value was wrong. It is better to assume success if the method does not return with a D-Bus error. On top of that, you can look at @failures to know whether any of the requested files failed. @failures: Paths of connection files that could not be loaded. Loads or reloads the indicated connections from disk. You should call this diff --git a/libnm/nm-client.c b/libnm/nm-client.c index d5ae5d4ac3..7785628624 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -1896,8 +1896,16 @@ nm_client_add_connection2_finish (NMClient *client, * then @failures will be set to a %NULL-terminated array of the * filenames that failed to load. * - * Returns: %TRUE if NetworkManager at least tried to load @filenames, - * %FALSE if an error occurred (eg, permission denied). + * Returns: %TRUE on success. + * + * Warning: before libnm 1.22, the boolean return value was inconsistent. + * That is made worse, because when running against certain server versions + * before 1.20, the server would return wrong values for success/failure. + * This means, if you use this function in libnm before 1.22, you are advised + * to ignore the boolean return value and only look at @failures and @error. + * With libnm >= 1.22, the boolean return value corresponds to whether @error was + * set. Note that even in the success case, you might have individual @failures. + * With 1.22, the return value is consistent with nm_client_load_connections_finish(). * * Deprecated: 1.22, use nm_client_load_connections_async() or GDBusConnection **/ @@ -1988,8 +1996,8 @@ nm_client_load_connections_async (NMClient *client, * See nm_client_load_connections() for more details. * - * Returns: %TRUE if NetworkManager at least tried to load @filenames, - * %FALSE if an error occurred (eg, permission denied). + * Returns: %TRUE on success. + * Note that even in the success case, you might have individual @failures. **/ gboolean nm_client_load_connections_finish (NMClient *client, @@ -2005,11 +2013,9 @@ nm_client_load_connections_finish (NMClient *client, simple = G_SIMPLE_ASYNC_RESULT (result); if (g_simple_async_result_propagate_error (simple, error)) return FALSE; - else { - if (failures) - *failures = g_strdupv (g_simple_async_result_get_op_res_gpointer (simple)); - return TRUE; - } + if (failures) + *failures = g_strdupv (g_simple_async_result_get_op_res_gpointer (simple)); + return TRUE; } /** diff --git a/libnm/nm-remote-settings.c b/libnm/nm-remote-settings.c index 24abcee397..3c6dc4ec93 100644 --- a/libnm/nm-remote-settings.c +++ b/libnm/nm-remote-settings.c @@ -416,9 +416,10 @@ nm_remote_settings_load_connections (NMRemoteSettings *settings, cancellable, error)) { if (error && *error) g_dbus_error_strip_remote_error (*error); - success = FALSE; + return FALSE; } - return success; + + return TRUE; } static void From 86097cc2e84989577bbcea44834f28e535e9d7d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 6 Oct 2019 23:19:32 +0200 Subject: [PATCH 09/47] libnm: fix return value for nm_remote_settings_reload_connections*() to ignore server result Note that the server always returns TRUE for the boolean return value of ReloadConnections. Hence, this should not change in behavior, because the server would never have returned FALSE. However, change behavior of the API. It's odd that the function might return %FALSE without setting the error output. It's also not clear what the boolean value of the "ReloadConnections" D-Bus would mean anyway. --- introspection/org.freedesktop.NetworkManager.Settings.xml | 2 +- libnm/nm-remote-settings.c | 6 +++--- src/settings/nm-settings.c | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/introspection/org.freedesktop.NetworkManager.Settings.xml b/introspection/org.freedesktop.NetworkManager.Settings.xml index 59e0d57549..0587db5f45 100644 --- a/introspection/org.freedesktop.NetworkManager.Settings.xml +++ b/introspection/org.freedesktop.NetworkManager.Settings.xml @@ -120,7 +120,7 @@