device: mark uses of device's state-reason with nm_device_state_reason_check()

The state-change of a device has a reason argument, which is mostly for information
only.

There are many places in code that are the source of a state-reason.
Mostly these are calls to:
  - nm_device_state_changed()
  - nm_device_queue_state()
  - nm_device_queue_recheck_available()
  - nm_device_set_unmanaged_by_*()
  - nm_device_master_release_one_slave()
  - nm_device_ip_method_failed()
  - nm_modem_emit_prepare_result()
  - nm_modem_emit_ppp_failed()
  - nm_manager_deactivate_connection()
  - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_*);

However, there are a few places in code that look at the reason
to decide how to proceed. I think this is a bad pattern, because
cause and effect are decoupled and it gets hard to understand where
a certain reason is set and what consequences that has.

Add a nop-function nm_device_state_reason_check() to mark all uses
of the device state reason that derive decisions from it. That is,
highlight the "effect" part.

10 files changed, 46 insertions, 24 deletions

diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c
index 717af06b1c..3bc67a559b 100644
--- a/src/devices/bluetooth/nm-device-bt.c
+++ b/src/devices/bluetooth/nm-device-bt.c
@@ -486,7 +486,7 @@ modem_prepare_result (NMModem *modem,
 			break;
 		}
 	} else {
-		if (reason == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
+		if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
 			/* If the connect failed because the SIM PIN was wrong don't allow
 			 * the device to be auto-activated anymore, which would risk locking
 			 * the SIM if the incorrect PIN continues to be used.
diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c
index e795d968df..9a8d847d6e 100644
--- a/src/devices/nm-device-macsec.c
+++ b/src/devices/nm-device-macsec.c
@@ -110,7 +110,7 @@ parent_state_changed (NMDevice *parent,
 	NMDeviceMacsec *self = NM_DEVICE_MACSEC (user_data);
 
 	/* We'll react to our own carrier state notifications. Ignore the parent's. */
-	if (reason == NM_DEVICE_STATE_REASON_CARRIER)
+	if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
 		return;
 
 	nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);
diff --git a/src/devices/nm-device-macvlan.c b/src/devices/nm-device-macvlan.c
index 383907765c..77adf28ec7 100644
--- a/src/devices/nm-device-macvlan.c
+++ b/src/devices/nm-device-macvlan.c
@@ -131,7 +131,7 @@ parent_state_changed (NMDevice *parent,
 	NMDeviceMacvlan *self = NM_DEVICE_MACVLAN (user_data);
 
 	/* We'll react to our own carrier state notifications. Ignore the parent's. */
-	if (reason == NM_DEVICE_STATE_REASON_CARRIER)
+	if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
 		return;
 
 	nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);
diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c
index c2f62f4096..2680353ca0 100644
--- a/src/devices/nm-device-vlan.c
+++ b/src/devices/nm-device-vlan.c
@@ -79,7 +79,7 @@ parent_state_changed (NMDevice *parent,
 	NMDeviceVlan *self = NM_DEVICE_VLAN (user_data);
 
 	/* We'll react to our own carrier state notifications. Ignore the parent's. */
-	if (reason == NM_DEVICE_STATE_REASON_CARRIER)
+	if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER)
 		return;
 
 	nm_device_set_unmanaged_by_flags (NM_DEVICE (self), NM_UNMANAGED_PARENT, !nm_device_get_managed (parent, FALSE), reason);
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index d12bdf752c..a0a5ed2aec 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2967,7 +2967,7 @@ slave_state_changed (NMDevice *slave,
 	}
 
 	/* Don't touch the device if its state changed externally. */
-	if (reason == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
+	if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
 		configure = FALSE;
 
 	if (release) {
@@ -3247,10 +3247,10 @@ nm_device_slave_notify_release (NMDevice *self, NMDeviceStateReason reason)
 
 	if (   priv->state > NM_DEVICE_STATE_DISCONNECTED
 	    && priv->state <= NM_DEVICE_STATE_ACTIVATED) {
-		if (reason == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) {
+		if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) {
 			new_state = NM_DEVICE_STATE_FAILED;
 			master_status = "failed";
-		} else if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
+		} else if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
 			new_state = NM_DEVICE_STATE_DEACTIVATING;
 			master_status = "deactivated by user request";
 		} else {
@@ -4232,7 +4232,6 @@ activate_stage1_device_prepare (NMDevice *self)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 	NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
-	NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
 	NMActiveConnection *active = NM_ACTIVE_CONNECTION (priv->act_request);
 
 	_set_ip_state (self, AF_INET, IP_NONE);
@@ -4246,6 +4245,8 @@ activate_stage1_device_prepare (NMDevice *self)
 
 	/* Assumed connections were already set up outside NetworkManager */
 	if (!nm_active_connection_get_assumed (active)) {
+		NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
+
 		ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &failure_reason);
 		if (ret == NM_ACT_STAGE_RETURN_POSTPONE) {
 			return;
@@ -11914,11 +11915,14 @@ _set_state_full (NMDevice *self,
 	case NM_DEVICE_STATE_UNMANAGED:
 		nm_device_set_firmware_missing (self, FALSE);
 		if (old_state > NM_DEVICE_STATE_UNMANAGED) {
-			if (reason == NM_DEVICE_STATE_REASON_REMOVED) {
+			switch (nm_device_state_reason_check (reason)) {
+			case NM_DEVICE_STATE_REASON_REMOVED:
 				nm_device_cleanup (self, reason, CLEANUP_TYPE_REMOVED);
-			} else if (reason == NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
+				break;
+			case NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED:
 				nm_device_cleanup (self, reason, CLEANUP_TYPE_KEEP);
-			} else {
+				break;
+			default:
 				/* Clean up if the device is now unmanaged but was activated */
 				if (nm_device_get_act_request (self))
 					nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
@@ -11926,17 +11930,18 @@ _set_state_full (NMDevice *self,
 				nm_device_hw_addr_reset (self, "unmanage");
 				set_nm_ipv6ll (self, FALSE);
 				restore_ip6_properties (self);
+				break;
 			}
 		}
 		break;
 	case NM_DEVICE_STATE_UNAVAILABLE:
 		if (old_state == NM_DEVICE_STATE_UNMANAGED) {
 			save_ip6_properties (self);
-			if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
+			if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED)
 				ip6_managed_setup (self);
 		}
 
-		if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
+		if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
 			if (old_state == NM_DEVICE_STATE_UNMANAGED || priv->firmware_missing) {
 				if (!nm_device_bring_up (self, TRUE, &no_firmware) && no_firmware)
 					_LOGW (LOGD_PLATFORM, "firmware may be missing.");
@@ -11963,7 +11968,7 @@ _set_state_full (NMDevice *self,
 
 			nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
 		} else if (old_state < NM_DEVICE_STATE_DISCONNECTED) {
-			if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
+			if (nm_device_state_reason_check (reason) != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
 				/* Ensure IPv6 is set up as it may not have been done when
 				 * entering the UNAVAILABLE state depending on the reason.
 				 */
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 955edcd7da..8a39fd7c5d 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -30,6 +30,21 @@
 #include "nm-rfkill-manager.h"
 #include "NetworkManagerUtils.h"
 
+static inline NMDeviceStateReason
+nm_device_state_reason_check (NMDeviceStateReason reason)
+{
+	/* the device-state-reason serves mostly informational purpse during a state
+	 * change. In some cases however, decisions are made based on the reason.
+	 * I tend to think that interpreting the state reason to derive some behaviors
+	 * is confusing, because the cause and effect are so far apart.
+	 *
+	 * This function is here to mark source that inspects the reason to make
+	 * a decision -- contrary to places that set the reason. Thus, by grepping
+	 * for nm_device_state_reason_check() you can find the "effect" to a certain
+	 * reason.
+	 */
+	return reason;
+}
 
 #define NM_PENDING_ACTION_AUTOACTIVATE              "autoactivate"
 #define NM_PENDING_ACTION_DHCP4                     "dhcp4"
diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c
index c9a7a2a814..4a4d2f2c3b 100644
--- a/src/devices/wwan/nm-device-modem.c
+++ b/src/devices/wwan/nm-device-modem.c
@@ -127,7 +127,7 @@ modem_prepare_result (NMModem *modem,
 	if (success)
 		nm_device_activate_schedule_stage2_device_config (device);
 	else {
-		if (reason == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
+		if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_SIM_PIN_INCORRECT) {
 			/* If the connect failed because the SIM PIN was wrong don't allow
 			 * the device to be auto-activated anymore, which would risk locking
 			 * the SIM if the incorrect PIN continues to be used.
@@ -377,7 +377,7 @@ device_state_changed (NMDevice *device,
 
 	nm_modem_device_state_changed (priv->modem, new_state, old_state);
 
-	switch (reason) {
+	switch (nm_device_state_reason_check (reason)) {
 	case NM_DEVICE_STATE_REASON_GSM_REGISTRATION_DENIED:
 	case NM_DEVICE_STATE_REASON_GSM_REGISTRATION_NOT_SEARCHING:
 	case NM_DEVICE_STATE_REASON_GSM_SIM_NOT_INSERTED:
diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c
index 5f11f739ce..ac8457edae 100644
--- a/src/devices/wwan/nm-modem-ofono.c
+++ b/src/devices/wwan/nm-modem-ofono.c
@@ -745,7 +745,6 @@ context_property_changed (GDBusProxy *proxy,
 {
 	NMModemOfono *self = NM_MODEM_OFONO (user_data);
 	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
-	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
 	NMPlatformIP4Address addr;
 	gboolean ret = FALSE;
 	GVariant *v_dict;
@@ -910,9 +909,10 @@ context_property_changed (GDBusProxy *proxy,
 out:
 	if (nm_modem_get_state (NM_MODEM (self)) != NM_MODEM_STATE_CONNECTED) {
 		_LOGI ("emitting PREPARE_RESULT: %s", ret ? "TRUE" : "FALSE");
-		if (!ret)
-			reason = NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE;
-		nm_modem_emit_prepare_result (NM_MODEM (self), ret, reason);
+		nm_modem_emit_prepare_result (NM_MODEM (self), ret,
+		                              ret
+		                                  ? NM_DEVICE_STATE_REASON_NONE
+		                                  : NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE);
 	} else {
 		_LOGW ("MODEM_PPP_FAILED");
 		nm_modem_emit_ppp_failed (NM_MODEM (self), NM_DEVICE_STATE_REASON_PPP_FAILED);
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 5588552389..2f7fc65e2b 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -3924,8 +3924,9 @@ nm_manager_deactivate_connection (NMManager *manager,
 	if (NM_IS_VPN_CONNECTION (active)) {
 		NMVpnConnectionStateReason vpn_reason = NM_VPN_CONNECTION_STATE_REASON_USER_DISCONNECTED;
 
-		if (reason == NM_DEVICE_STATE_REASON_CONNECTION_REMOVED)
+		if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CONNECTION_REMOVED)
 			vpn_reason = NM_VPN_CONNECTION_STATE_REASON_CONNECTION_REMOVED;
+
 		if (nm_vpn_connection_deactivate (NM_VPN_CONNECTION (active), vpn_reason, FALSE))
 			success = TRUE;
 		else
diff --git a/src/nm-policy.c b/src/nm-policy.c
index 9e6f51f652..bc602db72d 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -1465,7 +1465,7 @@ device_state_changed (NMDevice *device,
 		    && old_state <= NM_DEVICE_STATE_ACTIVATED) {
 			int tries = nm_settings_connection_get_autoconnect_retries (connection);
 
-			if (reason == NM_DEVICE_STATE_REASON_NO_SECRETS) {
+			if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) {
 				_LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets",
 				       nm_settings_connection_get_id (connection));
 
@@ -1524,7 +1524,7 @@ device_state_changed (NMDevice *device,
 			update_routing_and_dns (self, FALSE);
 		break;
 	case NM_DEVICE_STATE_DEACTIVATING:
-		if (reason == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
+		if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_USER_REQUESTED) {
 			if (!nm_device_get_autoconnect (device)) {
 				/* The device was disconnected; block all connections on it */
 				block_autoconnect_for_device (self, device);
@@ -1544,7 +1544,8 @@ device_state_changed (NMDevice *device,
 		/* Reset retry counts for a device's connections when carrier on; if cable
 		 * was unplugged and plugged in again, we should try to reconnect.
 		 */
-		if (reason == NM_DEVICE_STATE_REASON_CARRIER && old_state == NM_DEVICE_STATE_UNAVAILABLE)
+		if (   nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER
+		    && old_state == NM_DEVICE_STATE_UNAVAILABLE)
 			reset_autoconnect_all (self, device);
 
 		if (old_state > NM_DEVICE_STATE_DISCONNECTED)