core: infer the bluetooth type from the presence of the supplemental settings

When the user sets a GSM or CDMA setting along with a Bluetooth setting
we know we're dealing with a DUN profile. No need to ask.

[thaller@redhat.com: verify() and normalize() must strongly agree whether a
connection is normalizable, and now to do it. That is, after verify()
determines the connection is normalizable, normalize() must fix it as
anticipated.

The reason is, we only want to modify the connection, if we are able
to create a valid result. Hence, after normalize() it *must* verify().

Try to simplify that by moving the logic of fixing the bt-type to a
common place _nm_connection_detect_bluetooth_type().]

Co-Authored-By: Thomas Haller <thaller@redhat.com>

3 files changed, 89 insertions, 23 deletions

diff --git a/libnm-core/nm-connection-private.h b/libnm-core/nm-connection-private.h
index 620e6a81ec..7dd088ace0 100644
--- a/libnm-core/nm-connection-private.h
+++ b/libnm-core/nm-connection-private.h
@@ -29,6 +29,8 @@ NMSetting  *_nm_connection_find_base_type_setting         (NMConnection *connect
 const char *_nm_connection_detect_slave_type              (NMConnection *connection,
                                                            NMSetting **out_s_port);
 
+const char *_nm_connection_detect_bluetooth_type (NMConnection *self);
+
 gboolean    _nm_connection_verify_required_interface_name (NMConnection *connection,
                                                            GError **error);
 
diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index d2e528420e..a3fae442ba 100644
--- a/libnm-core/nm-connection.c
+++ b/libnm-core/nm-connection.c
@@ -684,6 +684,26 @@ _normalize_connection_type (NMConnection *self)
 }
 
 const char *
+_nm_connection_detect_bluetooth_type (NMConnection *self)
+{
+	NMSettingBluetooth *s_bt = nm_connection_get_setting_bluetooth (self);
+
+	if (   s_bt
+	    && nm_setting_bluetooth_get_connection_type (s_bt)) {
+		if (   nm_connection_get_setting_gsm (self)
+		    || nm_connection_get_setting_cdma (self))
+			return NM_SETTING_BLUETOOTH_TYPE_DUN;
+		if (nm_connection_get_setting_bridge (self))
+			return NM_SETTING_BLUETOOTH_TYPE_NAP;
+		return NM_SETTING_BLUETOOTH_TYPE_PANU;
+	}
+
+	/* NULL means the connection is not a bluetooth type, or it needs
+	 * no normalization, as the type is set explicitly. */
+	return NULL;
+}
+
+const char *
 _nm_connection_detect_slave_type (NMConnection *connection, NMSetting **out_s_port)
 {
 	NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (connection);
@@ -1053,6 +1073,20 @@ _normalize_team_port_config (NMConnection *self, GHashTable *parameters)
 }
 
 static gboolean
+_normalize_bluetooth_type (NMConnection *self, GHashTable *parameters)
+{
+	const char *type = _nm_connection_detect_bluetooth_type (self);
+
+	if (type) {
+		g_object_set (nm_connection_get_setting_bluetooth (self),
+		              NM_SETTING_BLUETOOTH_TYPE, type,
+		              NULL);
+		return TRUE;
+	}
+	return FALSE;
+}
+
+static gboolean
 _normalize_required_settings (NMConnection *self, GHashTable *parameters)
 {
 	NMSettingBluetooth *s_bt = nm_connection_get_setting_bluetooth (self);
@@ -1353,6 +1387,7 @@ nm_connection_normalize (NMConnection *connection,
 	was_modified |= _normalize_wireless_mac_address_randomization (connection, parameters);
 	was_modified |= _normalize_team_config (connection, parameters);
 	was_modified |= _normalize_team_port_config (connection, parameters);
+	was_modified |= _normalize_bluetooth_type (connection, parameters);
 
 	/* Verify anew. */
 	success = _nm_connection_verify (connection, error);
diff --git a/libnm-core/nm-setting-bluetooth.c b/libnm-core/nm-setting-bluetooth.c
index 6daf857ecb..5b6ef3eb97 100644
--- a/libnm-core/nm-setting-bluetooth.c
+++ b/libnm-core/nm-setting-bluetooth.c
@@ -113,6 +113,8 @@ static gboolean
 verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
 	NMSettingBluetoothPrivate *priv = NM_SETTING_BLUETOOTH_GET_PRIVATE (setting);
+	const char *type;
+	gboolean missing_nap_bridge = FALSE;
 
 	if (priv->bdaddr && !nm_utils_hwaddr_valid (priv->bdaddr, ETH_ALEN)) {
 		g_set_error_literal (error,
@@ -123,28 +125,38 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		return FALSE;
 	}
 
-	if (!priv->type) {
-		g_set_error_literal (error,
-		                     NM_CONNECTION_ERROR,
-		                     NM_CONNECTION_ERROR_MISSING_PROPERTY,
-		                     _("property is missing"));
-		g_prefix_error (error, "%s.%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BLUETOOTH_TYPE);
-		return FALSE;
-	} else if (!g_str_equal (priv->type, NM_SETTING_BLUETOOTH_TYPE_DUN) &&
-	           !g_str_equal (priv->type, NM_SETTING_BLUETOOTH_TYPE_NAP) &&
-	           !g_str_equal (priv->type, NM_SETTING_BLUETOOTH_TYPE_PANU)) {
+	type = priv->type;
+	if (!type) {
+		if (connection) {
+			/* We may infer the type from the (non-)existence of gsm/cdma/bridge settings. */
+			type = _nm_connection_detect_bluetooth_type (connection);
+		}
+		if (!type) {
+			g_set_error_literal (error,
+			                     NM_CONNECTION_ERROR,
+			                     NM_CONNECTION_ERROR_MISSING_PROPERTY,
+			                     _("property is missing"));
+			g_prefix_error (error, "%s.%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BLUETOOTH_TYPE);
+			return FALSE;
+		}
+	}
+
+	if (!NM_IN_STRSET (type, NM_SETTING_BLUETOOTH_TYPE_DUN,
+	                         NM_SETTING_BLUETOOTH_TYPE_NAP,
+	                         NM_SETTING_BLUETOOTH_TYPE_PANU)) {
+		nm_assert (priv->type == type);
 		g_set_error (error,
 		             NM_CONNECTION_ERROR,
 		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
 		             _("'%s' is not a valid value for the property"),
-		             priv->type);
+		             type);
 		g_prefix_error (error, "%s.%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BLUETOOTH_TYPE);
 		return FALSE;
 	}
 
 	/* Make sure the corresponding 'type' setting is present */
 	if (   connection
-	    && !strcmp (priv->type, NM_SETTING_BLUETOOTH_TYPE_DUN)) {
+	    && nm_streq (type, NM_SETTING_BLUETOOTH_TYPE_DUN)) {
 		gboolean gsm = FALSE, cdma = FALSE;
 
 		gsm = !!nm_connection_get_setting_gsm (connection);
@@ -170,19 +182,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	 */
 
 	/* NAP mode needs a bridge setting, and a bridge needs a name. */
-	if (!strcmp (priv->type, NM_SETTING_BLUETOOTH_TYPE_NAP)) {
+	if (nm_streq (type, NM_SETTING_BLUETOOTH_TYPE_NAP)) {
 		if (!_nm_connection_verify_required_interface_name (connection, error))
 			return FALSE;
-		if (connection && !nm_connection_get_setting_bridge (connection)) {
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_SETTING,
-			             _("'%s' connection requires '%s' setting"),
-			             NM_SETTING_BLUETOOTH_TYPE_NAP,
-			             NM_SETTING_BRIDGE_SETTING_NAME);
-			g_prefix_error (error, "%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME);
-			return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
-		}
+		if (   connection
+		    && !nm_connection_get_setting_bridge (connection))
+			missing_nap_bridge = TRUE;
 	} else {
 		if (!priv->bdaddr) {
 			g_set_error_literal (error,
@@ -194,6 +199,30 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
+	/* errors form here are normalizable. */
+
+	if (!priv->type) {
+		/* as determined above, we can detect the bluetooth type. */
+		nm_assert (!missing_nap_bridge);
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_MISSING_PROPERTY,
+		                     _("property is missing"));
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BLUETOOTH_TYPE);
+		return NM_SETTING_VERIFY_NORMALIZABLE;
+	}
+
+	if (missing_nap_bridge) {
+		g_set_error (error,
+		             NM_CONNECTION_ERROR,
+		             NM_CONNECTION_ERROR_INVALID_SETTING,
+		             _("'%s' connection requires '%s' setting"),
+		             NM_SETTING_BLUETOOTH_TYPE_NAP,
+		             NM_SETTING_BRIDGE_SETTING_NAME);
+		g_prefix_error (error, "%s: ", NM_SETTING_BLUETOOTH_SETTING_NAME);
+		return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
+	}
+
 	return TRUE;
 }