gsm,cdma: don't reset connected state if disconnecting fails

If disconnecting fails, we can't be sure we're not still connected,
which is why previously MM reset the CONNECTED state.  However, this
 behavior in conjunction with 27408b4c1e13e3016a01e5b273e66d99f487d3ad
caused ModemManager not to recognize that Bluetooth rfcomm ports have
disappeared.  Oddly udev doesn't tell us that they have gone away,
probably because ModemManager still has the port's file descriptor
open and virtual devices like rfcomm ports sometimes stick around
until all their users have disappeared.  Because of 27408b4c MM
wasn't listening to the serial port and the port hangup that would
normally cause MM to release the port was ignored.

But in the end, resetting CONNECTED state when disconnecting doesn't
do anything useful because it doesn't allow commands to be sent to
the modem which might (helpfully) interrupt the command stream and
get the device responsive.  Also, whatever terminated the data
connection and called Disconnect() should already have terminated
anything that would allow data to be transferred, like the PPP
session or cleared the IP addresses from the net interface, or
terminated the Bluetooth link.

3 files changed, 20 insertions, 114 deletions

diff --git a/src/mm-generic-cdma.c b/src/mm-generic-cdma.c
index 8a478c15..e15e0179 100644
--- a/src/mm-generic-cdma.c
+++ b/src/mm-generic-cdma.c
@@ -34,8 +34,6 @@
 #include "mm-log.h"
 #include "mm-utils.h"
 
-#define MM_GENERIC_CDMA_PREV_STATE_TAG "prev-state"
-
 typedef enum {
     RM_PROTO_ASYNC = 0,
     RM_PROTO_RELAY = 1,
@@ -724,38 +722,26 @@ enable (MMModem *modem,
 }
 
 static void
-disable_set_previous_state (MMModem *modem, MMCallbackInfo *info)
-{
-    MMModemState prev_state;
-
-    /* Reset old state since the operation failed */
-    prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_CDMA_PREV_STATE_TAG));
-    mm_modem_set_state (modem, prev_state, MM_MODEM_STATE_REASON_NONE);
-}
-
-static void
 disable_all_done (MMModem *modem, GError *error, gpointer user_data)
 {
     MMCallbackInfo *info = user_data;
+    MMGenericCdmaPrivate *priv;
 
     /* If the modem has already been removed, return without
      * scheduling callback */
     if (!modem || mm_callback_info_check_modem_removed (info))
         return;
 
-    if (error) {
+    priv = MM_GENERIC_CDMA_GET_PRIVATE (user_data);
+
+    if (error)
         info->error = g_error_copy (error);
-        disable_set_previous_state (modem, info);
-    } else {
-        MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
-        MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
 
-        mm_serial_port_close_force (MM_SERIAL_PORT (priv->primary));
-        mm_modem_set_state (modem, MM_MODEM_STATE_DISABLED, MM_MODEM_STATE_REASON_NONE);
+    mm_serial_port_close_force (MM_SERIAL_PORT (priv->primary));
+    mm_modem_set_state (modem, MM_MODEM_STATE_DISABLED, MM_MODEM_STATE_REASON_NONE);
 
-        priv->cdma_1x_reg_state = MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN;
-        priv->evdo_reg_state = MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN;
-    }
+    priv->cdma_1x_reg_state = MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN;
+    priv->evdo_reg_state = MM_MODEM_CDMA_REGISTRATION_STATE_UNKNOWN;
 
     mm_callback_info_schedule (info);
 }
@@ -773,14 +759,7 @@ disable_flash_done (MMSerialPort *port,
     if (mm_callback_info_check_modem_removed (info))
         return;
 
-    if (error) {
-        info->error = g_error_copy (error);
-
-        disable_set_previous_state (info->modem, info);
-        mm_callback_info_schedule (info);
-        return;
-    }
-
+    /* Ignore serial errors */
     self = MM_GENERIC_CDMA (info->modem);
 
     if (MM_GENERIC_CDMA_GET_CLASS (self)->post_disable)
@@ -797,20 +776,12 @@ disable (MMModem *modem,
     MMGenericCdma *self = MM_GENERIC_CDMA (modem);
     MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
     MMCallbackInfo *info;
-    MMModemState state;
 
     /* Tear down any ongoing registration */
     registration_cleanup (self, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL);
 
     info = mm_callback_info_new (modem, callback, user_data);
 
-    /* Cache the previous state so we can reset it if the operation fails */
-    state = mm_modem_get_state (modem);
-    mm_callback_info_set_data (info,
-                               MM_GENERIC_CDMA_PREV_STATE_TAG,
-                               GUINT_TO_POINTER (state),
-                               NULL);
-
     /* Close auxiliary serial ports */
     if (priv->secondary)
         mm_serial_port_close_force (MM_SERIAL_PORT (priv->secondary));
@@ -902,27 +873,18 @@ disconnect_flash_done (MMSerialPort *port,
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
     MMGenericCdma *self;
     MMGenericCdmaPrivate *priv;
-    MMModemState prev_state;
 
     /* If the modem has already been removed, return without
      * scheduling callback */
     if (mm_callback_info_check_modem_removed (info))
         return;
 
+    /* Ignore serial errors */
     self = MM_GENERIC_CDMA (info->modem);
     priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
-    if (error) {
-        info->error = g_error_copy (error);
 
-        /* Reset old state since the operation failed */
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_CDMA_PREV_STATE_TAG));
-        mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
-    } else {
-        mm_port_set_connected (priv->data, FALSE);
-        update_enabled_state (self, FALSE, MM_MODEM_STATE_REASON_NONE);
-    }
+    mm_port_set_connected (priv->data, FALSE);
+    update_enabled_state (self, FALSE, MM_MODEM_STATE_REASON_NONE);
 
     /* Balance any open from connect(); subclasses may not use the generic
      * class' connect function and so the dial port may not have been
@@ -945,20 +907,12 @@ disconnect (MMModem *modem,
 {
     MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (modem);
     MMCallbackInfo *info;
-    MMModemState state;
     MMAtSerialPort *dial_port;
 
     g_return_if_fail (priv->primary != NULL);
 
     info = mm_callback_info_new (modem, callback, user_data);
 
-    /* Cache the previous state so we can reset it if the operation fails */
-    state = mm_modem_get_state (modem);
-    mm_callback_info_set_data (info,
-                               MM_GENERIC_CDMA_PREV_STATE_TAG,
-                               GUINT_TO_POINTER (state),
-                               NULL);
-
     mm_modem_set_state (modem, MM_MODEM_STATE_DISCONNECTING, reason);
 
     dial_port = priv->primary;
diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c
index 754a0cac..0e7c30b3 100644
--- a/src/mm-generic-gsm.c
+++ b/src/mm-generic-gsm.c
@@ -2183,7 +2183,6 @@ disable_flash_done (MMSerialPort *port,
 {
     MMGenericGsmPrivate *priv;
     MMCallbackInfo *info = user_data;
-    MMModemState prev_state;
     char *cmd = NULL;
 
     /* If the modem has already been removed, return without
@@ -2191,19 +2190,7 @@ disable_flash_done (MMSerialPort *port,
     if (mm_callback_info_check_modem_removed (info))
         return;
 
-    if (error) {
-        info->error = g_error_copy (error);
-
-        /* Reset old state since the operation failed */
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
-        mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
-
-        mm_callback_info_schedule (info);
-        return;
-    }
-
+    /* Ignore serial errors */
     priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
 
     /* Disable unsolicited messages */
@@ -2270,7 +2257,6 @@ disable (MMModem *modem,
     MMGenericGsm *self = MM_GENERIC_GSM (modem);
     MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self);
     MMCallbackInfo *info;
-    MMModemState state;
 
     /* First, reset the previously used CID and clean up registration */
     g_warn_if_fail (priv->cid == -1);
@@ -2301,13 +2287,6 @@ disable (MMModem *modem,
 
     info = mm_callback_info_new (modem, callback, user_data);
 
-    /* Cache the previous state so we can reset it if the operation fails */
-    state = mm_modem_get_state (modem);
-    mm_callback_info_set_data (info,
-                               MM_GENERIC_GSM_PREV_STATE_TAG,
-                               GUINT_TO_POINTER (state),
-                               NULL);
-
     /* Clean up the secondary port if it's open */
     if (priv->secondary && mm_serial_port_is_open (MM_SERIAL_PORT (priv->secondary))) {
         mm_dbg("Shutting down secondary port");
@@ -3911,7 +3890,6 @@ disconnect_done (MMModem *modem,
                  gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    MMModemState prev_state;
     MMGenericGsmPrivate *priv;
 
     /* Do nothing if modem removed */
@@ -3919,18 +3897,12 @@ disconnect_done (MMModem *modem,
         return;
 
     priv = MM_GENERIC_GSM_GET_PRIVATE (modem);
-    if (error) {
+    if (error)
         info->error = g_error_copy (error);
-        /* Reset old state since the operation failed */
-        prev_state = GPOINTER_TO_UINT (mm_callback_info_get_data (info, MM_GENERIC_GSM_PREV_STATE_TAG));
-        mm_modem_set_state (MM_MODEM (info->modem),
-                            prev_state,
-                            MM_MODEM_STATE_REASON_NONE);
-    } else {
-        mm_port_set_connected (priv->data, FALSE);
-        priv->cid = -1;
-        mm_generic_gsm_update_enabled_state (MM_GENERIC_GSM (modem), FALSE, MM_MODEM_STATE_REASON_NONE);
-    }
+
+    mm_port_set_connected (priv->data, FALSE);
+    priv->cid = -1;
+    mm_generic_gsm_update_enabled_state (MM_GENERIC_GSM (modem), FALSE, MM_MODEM_STATE_REASON_NONE);
 
     /* Balance any open from connect(); subclasses may not use the generic
      * class' connect function and so the dial port may not have been
@@ -3995,19 +3967,9 @@ disconnect_flash_done (MMSerialPort *port,
     if (mm_callback_info_check_modem_removed (info))
         return;
 
-    if (error) {
-        /* Ignore "NO CARRIER" response when modem disconnects and any flash
-         * failures we might encounter.  Other errors are hard errors.
-         */
-        if (   !g_error_matches (error, MM_MODEM_CONNECT_ERROR, MM_MODEM_CONNECT_ERROR_NO_CARRIER)
-            && !g_error_matches (error, MM_SERIAL_ERROR, MM_SERIAL_ERROR_FLASH_FAILED)) {
-            info->error = g_error_copy (error);
-            mm_callback_info_schedule (info);
-            return;
-        }
-    }
-
+    /* Ignore serial errors */
     priv = MM_GENERIC_GSM_GET_PRIVATE (info->modem);
+
     mm_port_set_connected (priv->data, FALSE);
 
     /* Don't bother doing the CGACT again if it was done on a secondary port,
@@ -4098,19 +4060,11 @@ disconnect (MMModem *modem,
     MMGenericGsm *self = MM_GENERIC_GSM (modem);
     MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self);
     MMCallbackInfo *info;
-    MMModemState state;
 
     priv->roam_allowed = TRUE;
 
     info = mm_callback_info_new (modem, callback, user_data);
 
-    /* Cache the previous state so we can reset it if the operation fails */
-    state = mm_modem_get_state (modem);
-    mm_callback_info_set_data (info,
-                               MM_GENERIC_GSM_PREV_STATE_TAG,
-                               GUINT_TO_POINTER (state),
-                               NULL);
-
     mm_modem_set_state (modem, MM_MODEM_STATE_DISCONNECTING, reason);
 
     g_assert (MM_GENERIC_GSM_GET_CLASS (self)->do_disconnect);
diff --git a/src/mm-generic-gsm.h b/src/mm-generic-gsm.h
index a76d8b5f..b4055b0e 100644
--- a/src/mm-generic-gsm.h
+++ b/src/mm-generic-gsm.h
@@ -202,8 +202,6 @@ MMModem *mm_generic_gsm_new (const char *device,
 
 /* Private, for subclasses */
 
-#define MM_GENERIC_GSM_PREV_STATE_TAG "prev-state"
-
 void mm_generic_gsm_pending_registration_stop (MMGenericGsm *modem);
 
 void mm_generic_gsm_ussd_cleanup (MMGenericGsm *modem);