core: refcount serial port open/close

This specifically fixes a regression with Novatel GSM secondary
AT port enablement, where the inital pin check closed the port
before the Novatel plugin could send the command to flip secondary
ports to AT mode.

But it's useful elsewhere too, and simplifies a bunch of the PIN
checking code which had to use various ugly methods to track whether
to close the port or not after checking the PIN.

4 files changed, 103 insertions, 73 deletions

diff --git a/src/mm-generic-cdma.c b/src/mm-generic-cdma.c
index 5cad9732..7c42e81f 100644
--- a/src/mm-generic-cdma.c
+++ b/src/mm-generic-cdma.c
@@ -614,7 +614,7 @@ disable_all_done (MMModem *modem, GError *error, gpointer user_data)
         MMGenericCdma *self = MM_GENERIC_CDMA (info->modem);
         MMGenericCdmaPrivate *priv = MM_GENERIC_CDMA_GET_PRIVATE (self);
 
-        mm_serial_port_close (MM_SERIAL_PORT (priv->primary));
+        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;
@@ -672,9 +672,9 @@ disable (MMModem *modem,
 
     /* Close auxiliary serial ports */
     if (priv->secondary)
-        mm_serial_port_close (MM_SERIAL_PORT (priv->secondary));
+        mm_serial_port_close_force (MM_SERIAL_PORT (priv->secondary));
     if (priv->qcdm)
-        mm_serial_port_close (MM_SERIAL_PORT (priv->qcdm));
+        mm_serial_port_close_force (MM_SERIAL_PORT (priv->qcdm));
 
     mm_modem_set_state (MM_MODEM (info->modem),
                         MM_MODEM_STATE_DISABLING,
diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c
index a80f7680..3a3413e6 100644
--- a/src/mm-generic-gsm.c
+++ b/src/mm-generic-gsm.c
@@ -322,7 +322,7 @@ pin_check_again (gpointer user_data)
     MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self);
 
     priv->pin_check_timeout = 0;
-    check_pin (self, initial_pin_check_done, GUINT_TO_POINTER (TRUE));
+    check_pin (self, initial_pin_check_done, NULL);
     return FALSE;
 }
 
@@ -330,7 +330,6 @@ static void
 initial_pin_check_done (MMModem *modem, GError *error, gpointer user_data)
 {
     MMGenericGsmPrivate *priv;
-    gboolean close_port = !!user_data;
 
     /* modem could have been removed before we get here, in which case
      * 'modem' will be NULL.
@@ -350,8 +349,7 @@ initial_pin_check_done (MMModem *modem, GError *error, gpointer user_data)
         priv->pin_check_timeout = g_timeout_add_seconds (2, pin_check_again, modem);
     } else {
         priv->pin_checked = TRUE;
-        if (close_port)
-            mm_serial_port_close (MM_SERIAL_PORT (priv->primary));
+        mm_serial_port_close (MM_SERIAL_PORT (priv->primary));
         check_valid (MM_GENERIC_GSM (modem));
     }
 }
@@ -368,7 +366,7 @@ initial_pin_check (MMGenericGsm *self)
     g_return_if_fail (priv->primary != NULL);
 
     if (mm_serial_port_open (MM_SERIAL_PORT (priv->primary), &error))
-        check_pin (self, initial_pin_check_done, GUINT_TO_POINTER (TRUE));
+        check_pin (self, initial_pin_check_done, NULL);
     else {
         g_warning ("%s: failed to open serial port: (%d) %s",
                    __func__,
@@ -379,7 +377,7 @@ initial_pin_check (MMGenericGsm *self)
         /* Ensure the modem is still somewhat usable if opening the serial
          * port fails for some reason.
          */
-        initial_pin_check_done (MM_MODEM (self), NULL, GUINT_TO_POINTER (FALSE));
+        initial_pin_check_done (MM_MODEM (self), NULL, NULL);
     }
 }
 
@@ -679,9 +677,9 @@ enable_failed (MMModem *modem, GError *error, MMCallbackInfo *info)
         priv = MM_GENERIC_GSM_GET_PRIVATE (modem);
 
         if (priv->primary && mm_serial_port_is_open (MM_SERIAL_PORT (priv->primary)))
-            mm_serial_port_close (MM_SERIAL_PORT (priv->primary));
+            mm_serial_port_close_force (MM_SERIAL_PORT (priv->primary));
         if (priv->secondary && mm_serial_port_is_open (MM_SERIAL_PORT (priv->secondary)))
-            mm_serial_port_close (MM_SERIAL_PORT (priv->secondary));
+            mm_serial_port_close_force (MM_SERIAL_PORT (priv->secondary));
     }
 
     mm_callback_info_schedule (info);
@@ -968,7 +966,7 @@ disable_done (MMAtSerialPort *port,
     if (!info->error) {
         MMGenericGsm *self = MM_GENERIC_GSM (info->modem);
 
-        mm_serial_port_close (MM_SERIAL_PORT (port));
+        mm_serial_port_close_force (MM_SERIAL_PORT (port));
         mm_modem_set_state (MM_MODEM (info->modem),
                             MM_MODEM_STATE_DISABLED,
                             MM_MODEM_STATE_REASON_NONE);
@@ -1056,7 +1054,7 @@ disable (MMModem *modem,
 
     /* Close the secondary port if its open */
     if (priv->secondary && mm_serial_port_is_open (MM_SERIAL_PORT (priv->secondary)))
-        mm_serial_port_close (MM_SERIAL_PORT (priv->secondary));
+        mm_serial_port_close_force (MM_SERIAL_PORT (priv->secondary));
 
     info = mm_callback_info_new (modem, callback, user_data);
 
@@ -1130,7 +1128,6 @@ get_card_info (MMModem *modem,
     g_clear_error (&error);
 }
 
-#define PIN_CLOSE_PORT_TAG "pin-close-port"
 #define PIN_PORT_TAG "pin-port"
 
 static void
@@ -1150,7 +1147,7 @@ static void
 pin_puk_recheck_done (MMModem *modem, GError *error, gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    gboolean close_port = !!mm_callback_info_get_data (info, PIN_CLOSE_PORT_TAG);
+    MMSerialPort *port;
 
     /* Clear the pin check timeout to ensure that it won't ever get a
      * stale MMCallbackInfo if the modem got removed.  We'll reschedule it here
@@ -1186,13 +1183,10 @@ pin_puk_recheck_done (MMModem *modem, GError *error, gpointer user_data)
         }
     }
 
-    /* Otherwise, clean up and return the PIN check error */
-    if (modem && close_port) {
-        MMSerialPort *port = mm_callback_info_get_data (info, PIN_PORT_TAG);
-
-        if (port && MM_IS_SERIAL_PORT (port))
-            mm_serial_port_close (port);
-    }
+    /* Otherwise, clean up and return the PIN check result */
+    port = mm_callback_info_get_data (info, PIN_PORT_TAG);
+    if (modem && port)
+        mm_serial_port_close (port);
 
     mm_callback_info_schedule (info);
 }
@@ -1204,13 +1198,11 @@ send_puk_done (MMAtSerialPort *port,
                gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    gboolean close_port = !!mm_callback_info_get_data (info, PIN_CLOSE_PORT_TAG);
 
     if (error) {
         info->error = g_error_copy (error);
         mm_callback_info_schedule (info);
-        if (close_port)
-            mm_serial_port_close (MM_SERIAL_PORT (port));
+        mm_serial_port_close (MM_SERIAL_PORT (port));
         return;
     }
 
@@ -1239,19 +1231,15 @@ send_puk (MMModemGsmCard *modem,
         return;
     }
 
-    if (!mm_serial_port_is_open (MM_SERIAL_PORT (port))) {
-        /* Modem may not be enabled yet, which sometimes can't be done until
-         * the device has been unlocked.
-         */
-        if (!mm_serial_port_open (MM_SERIAL_PORT (port), &info->error)) {
-            mm_callback_info_schedule (info);
-            return;
-        }
-
-        /* Clean up after ourselves if we opened the port */
-        mm_callback_info_set_data (info, PIN_CLOSE_PORT_TAG, GUINT_TO_POINTER (TRUE), NULL);
-        mm_callback_info_set_data (info, PIN_PORT_TAG, port, NULL);
+    /* Modem may not be enabled yet, which sometimes can't be done until
+     * the device has been unlocked.  In this case we have to open the port
+     * ourselves.
+     */
+    if (!mm_serial_port_open (MM_SERIAL_PORT (port), &info->error)) {
+        mm_callback_info_schedule (info);
+        return;
     }
+    mm_callback_info_set_data (info, PIN_PORT_TAG, port, NULL);
 
     command = g_strdup_printf ("+CPIN=\"%s\",\"%s\"", puk, pin);
     mm_at_serial_port_queue_command (port, command, 3, send_puk_done, info);
@@ -1265,13 +1253,11 @@ send_pin_done (MMAtSerialPort *port,
                gpointer user_data)
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
-    gboolean close_port = !!mm_callback_info_get_data (info, PIN_CLOSE_PORT_TAG);
 
     if (error) {
         info->error = g_error_copy (error);
         mm_callback_info_schedule (info);
-        if (close_port)
-            mm_serial_port_close (MM_SERIAL_PORT (port));
+        mm_serial_port_close (MM_SERIAL_PORT (port));
         return;
     }
 
@@ -1299,19 +1285,15 @@ send_pin (MMModemGsmCard *modem,
         return;
     }
 
-    if (!mm_serial_port_is_open (MM_SERIAL_PORT (port))) {
-        /* Modem may not be enabled yet, which sometimes can't be done until
-         * the device has been unlocked.
-         */
-        if (!mm_serial_port_open (MM_SERIAL_PORT (port), &info->error)) {
-            mm_callback_info_schedule (info);
-            return;
-        }
-
-        /* Clean up after ourselves if we opened the port */
-        mm_callback_info_set_data (info, PIN_CLOSE_PORT_TAG, GUINT_TO_POINTER (TRUE), NULL);
-        mm_callback_info_set_data (info, PIN_PORT_TAG, port, NULL);
+    /* Modem may not be enabled yet, which sometimes can't be done until
+     * the device has been unlocked.  In this case we have to open the port
+     * ourselves.
+     */
+    if (!mm_serial_port_open (MM_SERIAL_PORT (port), &info->error)) {
+        mm_callback_info_schedule (info);
+        return;
     }
+    mm_callback_info_set_data (info, PIN_PORT_TAG, port, NULL);
 
     command = g_strdup_printf ("+CPIN=\"%s\"", pin);
     mm_at_serial_port_queue_command (port, command, 3, send_pin_done, info);
diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
index b7fdf275..37f1bb09 100644
--- a/src/mm-serial-port.c
+++ b/src/mm-serial-port.c
@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 - 2009 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #define _GNU_SOURCE  /* for strcasestr() */
@@ -51,6 +51,7 @@ enum {
 #define MM_SERIAL_PORT_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), MM_TYPE_SERIAL_PORT, MMSerialPortPrivate))
 
 typedef struct {
+    guint32 open_count;
     int fd;
     GHashTable *reply_cache;
     GIOChannel *channel;
@@ -601,7 +602,7 @@ data_available (GIOChannel *source,
     if (condition & G_IO_HUP) {
         if (priv->response->len)
             g_byte_array_remove_range (priv->response, 0, priv->response->len);
-        mm_serial_port_close (self);
+        mm_serial_port_close_force (self);
         return FALSE;
     }
 
@@ -688,13 +689,13 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
 
     priv = MM_SERIAL_PORT_GET_PRIVATE (self);
 
-    if (priv->fd >= 0) {
+    device = mm_port_get_device (MM_PORT (self));
+
+    if (priv->open_count) {
         /* Already open */
-        return TRUE;
+        goto success;
     }
 
-    device = mm_port_get_device (MM_PORT (self));
-
     g_message ("(%s) opening serial device...", device);
     devfile = g_strdup_printf ("/dev/%s", device);
     errno = 0;
@@ -716,9 +717,7 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
     if (ioctl (priv->fd, TIOCEXCL) < 0) {
         g_set_error (error, MM_SERIAL_ERROR, MM_SERIAL_ERROR_OPEN_FAILED,
                      "Could not lock serial device %s: %s", device, strerror (errno));
-        close (priv->fd);
-        priv->fd = -1;
-        return FALSE;
+        goto error;
     }
 
     /* Flush any waiting IO */
@@ -727,17 +726,12 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
     if (tcgetattr (priv->fd, &priv->old_t) < 0) {
         g_set_error (error, MM_SERIAL_ERROR, MM_SERIAL_ERROR_OPEN_FAILED,
                      "Could not open serial device %s: %s", device, strerror (errno));
-        close (priv->fd);
-        priv->fd = -1;
-        return FALSE;
+        goto error;
     }
 
     g_warn_if_fail (MM_SERIAL_PORT_GET_CLASS (self)->config_fd);
-    if (!MM_SERIAL_PORT_GET_CLASS (self)->config_fd (self, priv->fd, error)) {
-        close (priv->fd);
-        priv->fd = -1;
-        return FALSE;
-    }
+    if (!MM_SERIAL_PORT_GET_CLASS (self)->config_fd (self, priv->fd, error))
+        goto error;
 
     priv->channel = g_io_channel_unix_new (priv->fd);
     g_io_channel_set_encoding (priv->channel, NULL, NULL);
@@ -749,7 +743,21 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
     priv->connected_id = g_signal_connect (self, "notify::" MM_PORT_CONNECTED,
                                            G_CALLBACK (port_connected), NULL);
 
+success:
+    priv->open_count++;
+    if (mm_options_debug ()) {
+        GTimeVal tv;
+
+        g_get_current_time (&tv);
+        g_debug ("<%ld.%ld> (%s) device open count is %d (open)",
+                 tv.tv_sec, tv.tv_usec, device, priv->open_count);
+    }
     return TRUE;
+
+error:
+    close (priv->fd);
+    priv->fd = -1;
+    return FALSE;
 }
 
 gboolean
@@ -758,18 +766,35 @@ mm_serial_port_is_open (MMSerialPort *self)
     g_return_val_if_fail (self != NULL, FALSE);
     g_return_val_if_fail (MM_IS_SERIAL_PORT (self), FALSE);
 
-    return (MM_SERIAL_PORT_GET_PRIVATE (self)->fd >= 0);
+    return !!MM_SERIAL_PORT_GET_PRIVATE (self)->open_count;
 }
 
 void
 mm_serial_port_close (MMSerialPort *self)
 {
     MMSerialPortPrivate *priv;
+    const char *device;
     int i;
 
     g_return_if_fail (MM_IS_SERIAL_PORT (self));
 
     priv = MM_SERIAL_PORT_GET_PRIVATE (self);
+    g_return_if_fail (priv->open_count > 0);
+
+    device = mm_port_get_device (MM_PORT (self));
+
+    priv->open_count--;
+
+    if (mm_options_debug ()) {
+        GTimeVal tv;
+
+        g_get_current_time (&tv);
+        g_debug ("<%ld.%ld> (%s) device open count is %d (close)",
+                 tv.tv_sec, tv.tv_usec, device, priv->open_count);
+    }
+
+    if (priv->open_count > 0)
+        return;
 
     if (priv->connected_id) {
         g_signal_handler_disconnect (self, priv->connected_id);
@@ -777,7 +802,7 @@ mm_serial_port_close (MMSerialPort *self)
     }
 
     if (priv->fd >= 0) {
-        g_message ("(%s) closing serial device...", mm_port_get_device (MM_PORT (self)));
+        g_message ("(%s) closing serial device...", device);
 
         mm_port_set_connected (MM_PORT (self), FALSE);
 
@@ -809,6 +834,22 @@ mm_serial_port_close (MMSerialPort *self)
     g_queue_clear (priv->queue);
 }
 
+void
+mm_serial_port_close_force (MMSerialPort *self)
+{
+    MMSerialPortPrivate *priv;
+
+    g_return_if_fail (self != NULL);
+    g_return_if_fail (MM_IS_SERIAL_PORT (self));
+
+    priv = MM_SERIAL_PORT_GET_PRIVATE (self);
+    g_return_if_fail (priv->open_count > 0);
+
+    /* Force the port to close */
+    priv->open_count = 1;
+    mm_serial_port_close (self);
+}
+
 static void
 internal_queue_command (MMSerialPort *self,
                         GByteArray *command,
@@ -1170,7 +1211,8 @@ get_property (GObject *object, guint prop_id,
 static void
 dispose (GObject *object)
 {
-    mm_serial_port_close (MM_SERIAL_PORT (object));
+    if (mm_serial_port_is_open (MM_SERIAL_PORT (object)))
+        mm_serial_port_close_force (MM_SERIAL_PORT (object));
 
     G_OBJECT_CLASS (mm_serial_port_parent_class)->dispose (object);
 }
diff --git a/src/mm-serial-port.h b/src/mm-serial-port.h
index 4da2b23a..f78f7931 100644
--- a/src/mm-serial-port.h
+++ b/src/mm-serial-port.h
@@ -11,7 +11,7 @@
  * GNU General Public License for more details:
  *
  * Copyright (C) 2008 - 2009 Novell, Inc.
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009 - 2010 Red Hat, Inc.
  */
 
 #ifndef MM_SERIAL_PORT_H
@@ -101,6 +101,10 @@ GType mm_serial_port_get_type (void);
 
 MMSerialPort *mm_serial_port_new (const char *name, MMPortType ptype);
 
+/* Keep in mind that port open/close is refcounted, so ensure that
+ * open/close calls are properly balanced.
+ */
+
 gboolean mm_serial_port_is_open           (MMSerialPort *self);
 
 gboolean mm_serial_port_open              (MMSerialPort *self,
@@ -108,6 +112,8 @@ gboolean mm_serial_port_open              (MMSerialPort *self,
 
 void     mm_serial_port_close             (MMSerialPort *self);
 
+void     mm_serial_port_close_force       (MMSerialPort *self);
+
 gboolean mm_serial_port_flash             (MMSerialPort *self,
                                            guint32 flash_time,
                                            gboolean ignore_errors,