From 57e140d961604e5d33a4a7a73210b3ece77c6e4b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 7 Jan 2025 08:33:29 +0100 Subject: [PATCH] manager: split device creation off from validate_activation_request() Make validate_activation_request() only do the validation -- split the determination of the device into find_device_for_activation(). The point of this is to be able complete the connection and actually create a virtual device after the validation. I believe this is also somewhat easier to follow now that the procedure does what its name says. --- src/core/nm-manager.c | 107 +++++++++++++++++++++++++++--------------- 1 file changed, 70 insertions(+), 37 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index e23cf0cabf..8f77165bf6 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -6403,17 +6403,11 @@ nm_manager_activate_connection(NMManager *self, * @sett_conn: the #NMSettingsConnection to be activated, or %NULL if there * is only a partial activation. * @connection: the partial #NMConnection to be activated (if @sett_conn is unspecified) - * @device_path: the object path of the device to be activated, or NULL - * @out_device: on successful return, the #NMDevice to be activated with @connection - * The caller may pass in a device which shortcuts the lookup by path. - * In this case, the passed in device must have the matching @device_path - * already. - * @out_is_vpn: on successful return, %TRUE if @connection is a VPN connection * @error: location to store an error on failure * - * Performs basic validation on an activation request, including ensuring that - * the requestor is a valid Unix process, is not disallowed in @connection - * permissions, and that a device exists that can activate @connection. + * Performs basic permission validation on an activation request: Ensures that + * the requestor is a valid Unix process and is not disallowed in @connection + * permissions. * * Returns: on success, the #NMAuthSubject representing the requestor, or * %NULL on error @@ -6423,13 +6417,8 @@ validate_activation_request(NMManager *self, GDBusMethodInvocation *context, NMSettingsConnection *sett_conn, NMConnection *connection, - const char *device_path, - NMDevice **out_device, - gboolean *out_is_vpn, GError **error) { - NMDevice *device = NULL; - gboolean is_vpn = FALSE; gs_unref_object NMAuthSubject *subject = NULL; nm_assert(!sett_conn || NM_IS_SETTINGS_CONNECTION(sett_conn)); @@ -6437,8 +6426,6 @@ validate_activation_request(NMManager *self, nm_assert(sett_conn || connection); nm_assert(!connection || !sett_conn || connection == nm_settings_connection_get_connection(sett_conn)); - nm_assert(out_device); - nm_assert(out_is_vpn); if (!connection) connection = nm_settings_connection_get_connection(sett_conn); @@ -6459,6 +6446,51 @@ validate_activation_request(NMManager *self, NM_MANAGER_ERROR_PERMISSION_DENIED, error)) return NULL; + return g_steal_pointer(&subject); +} + +/** + * find_device_for_activation: + * @self: the #NMManager + * @sett_conn: the #NMSettingsConnection to be activated, or %NULL if there + * is only a partial activation. + * @connection: the partial #NMConnection to be activated (if @sett_conn is unspecified) + * @device_path: the object path of the device to be activated, or NULL + * @out_device: on successful return, the #NMDevice to be activated with @connection + * The caller may pass in a device which shortcuts the lookup by path. + * In this case, the passed in device must have the matching @device_path + * already. + * @out_is_vpn: on successful return, %TRUE if @connection is a VPN connection + * @error: location to store an error on failure + * + * Looks up a device that can activate @connection, or indicates the + * connection is a VPN connection that does not require a device. + * + * Returns: %TRUE if the device could be find or connection doesn't + * need one, %FALSE otherwise + */ +static gboolean +find_device_for_activation(NMManager *self, + NMSettingsConnection *sett_conn, + NMConnection *connection, + const char *device_path, + NMDevice **out_device, + gboolean *out_is_vpn, + GError **error) +{ + gboolean is_vpn = FALSE; + NMDevice *device = NULL; + + nm_assert(!sett_conn || NM_IS_SETTINGS_CONNECTION(sett_conn)); + nm_assert(!connection || NM_IS_CONNECTION(connection)); + nm_assert(sett_conn || connection); + nm_assert(!connection || !sett_conn + || connection == nm_settings_connection_get_connection(sett_conn)); + nm_assert(out_device); + nm_assert(out_is_vpn); + + if (!connection) + connection = nm_settings_connection_get_connection(sett_conn); is_vpn = _connection_is_vpn(connection); @@ -6475,7 +6507,7 @@ validate_activation_request(NMManager *self, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, "Device not found"); - return NULL; + return FALSE; } } else if (!is_vpn) { gs_free_error GError *local = NULL; @@ -6497,13 +6529,13 @@ validate_activation_request(NMManager *self, NM_MANAGER_ERROR_UNKNOWN_DEVICE, "No suitable device found for this connection (%s).", local->message); - return NULL; + return FALSE; } /* Look for an existing device with the connection's interface name */ iface = nm_manager_get_connection_iface(self, connection, NULL, NULL, error); if (!iface) - return NULL; + return FALSE; device = find_device_by_iface(self, iface, connection, NULL, NULL); if (!device) { @@ -6511,7 +6543,7 @@ validate_activation_request(NMManager *self, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, "Failed to find a compatible device for this connection"); - return NULL; + return FALSE; } } } @@ -6520,7 +6552,8 @@ validate_activation_request(NMManager *self, *out_device = device; *out_is_vpn = is_vpn; - return g_steal_pointer(&subject); + + return TRUE; } /*****************************************************************************/ @@ -6637,17 +6670,14 @@ impl_manager_activate_connection(NMDBusObject *obj, goto error; } - subject = validate_activation_request(self, - invocation, - sett_conn, - NULL, - device_path, - &device, - &is_vpn, - &error); + subject = validate_activation_request(self, invocation, sett_conn, NULL, &error); if (!subject) goto error; + if (!find_device_for_activation(self, sett_conn, NULL, device_path, &device, &is_vpn, &error)) { + goto error; + } + active = _new_active_connection(self, is_vpn, sett_conn, @@ -6920,17 +6950,20 @@ impl_manager_add_and_activate_connection(NMDBusObject *obj, NM_SETTING_PARSE_FLAGS_STRICT, NULL); - subject = validate_activation_request(self, - invocation, - NULL, - incompl_conn, - device_path, - &device, - &is_vpn, - &error); + subject = validate_activation_request(self, invocation, NULL, incompl_conn, &error); if (!subject) goto error; + if (!find_device_for_activation(self, + NULL, + incompl_conn, + device_path, + &device, + &is_vpn, + &error)) { + goto error; + } + if (is_vpn) { /* Try to fill the VPN's connection setting and name at least */ if (!nm_connection_get_setting_vpn(incompl_conn)) {