GNOME.org
 

[PATCH] ModemManager: Clean up GRegex/GMatchInfo leaks

During some other development we found that there was a lot of
inconsistency in freeing GMatchInfo structures after g_regex
operations, and that quite a few match_info structures were probably
being leaked. This patch is the result of a quick audit of the g_regex
callers for properly cleaning up the match_info.

    - Nathan

From a090d2f3d99a1bd52fdd652b043e66e7dc48bb2c Mon Sep 17 00:00:00 2001
From: Nathan Williams <njw chromium org>
Date: Mon, 26 Sep 2011 13:20:17 -0400
Subject: [PATCH] Ensure that GMatchInfo and GRegex objects are freed properly.

In particular, g_regex_match() and g_regex_match_full() allocate a
match_info structure on both success and failure, so calling
g_match_info_free() only in the success case is insufficient.

BUG=None
TEST=Inspection

Change-Id: Iea76b5b5dc3ec48120e15601a5e2dd45322133d8
---
 plugins/mm-modem-anydata-cdma.c  |    6 ++++++
 plugins/mm-modem-cinterion-gsm.c |    9 +++++++++
 plugins/mm-modem-huawei-gsm.c    |    3 ++-
 plugins/mm-modem-novatel-gsm.c   |    3 +--
 plugins/mm-modem-samsung-gsm.c   |    3 ++-
 plugins/mm-modem-sierra-gsm.c    |    4 +++-
 plugins/mm-modem-x22x-gsm.c      |    3 +--
 plugins/mm-modem-zte.c           |    6 +++---
 src/mm-generic-gsm.c             |    4 ++++
 src/mm-modem-helpers.c           |   15 ++++++---------
 src/mm-serial-parsers.c          |   31 +++++++++++++------------------
 11 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/plugins/mm-modem-anydata-cdma.c b/plugins/mm-modem-anydata-cdma.c
index 7b6b37a..d26d3ec 100644
--- a/plugins/mm-modem-anydata-cdma.c
+++ b/plugins/mm-modem-anydata-cdma.c
@@ -183,6 +183,9 @@ evdo_state_done (MMAtSerialPort *port,
         }
     }
 
+    g_match_info_free (match_info);
+    g_regex_unref (r);
+
 done:
     mm_generic_cdma_query_reg_state_set_callback_evdo_state (info, reg_state);
     mm_callback_info_schedule (info);
@@ -254,6 +257,9 @@ state_done (MMAtSerialPort *port,
         }
     }
 
+    g_match_info_free (match_info);
+    g_regex_unref (r);
+
     mm_generic_cdma_query_reg_state_set_callback_1x_state (info, reg_state);
 
     /* Try for EVDO state too */
diff --git a/plugins/mm-modem-cinterion-gsm.c b/plugins/mm-modem-cinterion-gsm.c
index 56a6b00..b6ea841 100644
--- a/plugins/mm-modem-cinterion-gsm.c
+++ b/plugins/mm-modem-cinterion-gsm.c
@@ -615,6 +615,9 @@ get_smong_cb (MMAtSerialPort *port,
         priv->sind_psinfo = TRUE;
     }
 
+    g_match_info_free (match_info);
+    g_regex_unref (regex);
+
     mm_callback_info_set_result (info, GUINT_TO_POINTER (act), NULL);
     mm_callback_info_schedule (info);
 }
@@ -661,9 +664,15 @@ get_sind_cb (MMAtSerialPort *port,
         g_free (ind_value);
         mm_callback_info_set_result (info, GUINT_TO_POINTER (act), NULL);
         mm_callback_info_schedule (info);
+
+        g_match_info_free (match_info);
+        g_regex_unref (regex);
         return;
     }
 
+    g_match_info_free (match_info);
+    g_regex_unref (regex);
+
     /* If there was no 'psinfo' indicator, we'll try AT^SMONG and read the cell
      * info table. */
     mm_at_serial_port_queue_command (port, "^SMONG", 3, get_smong_cb, info);
diff --git a/plugins/mm-modem-huawei-gsm.c b/plugins/mm-modem-huawei-gsm.c
index e038069..f615e19 100644
--- a/plugins/mm-modem-huawei-gsm.c
+++ b/plugins/mm-modem-huawei-gsm.c
@@ -595,9 +595,10 @@ send_huawei_cpin_done (MMAtSerialPort *port,
 
     mm_callback_info_set_result (info, GUINT_TO_POINTER (attempts_left), NULL);
 
-    g_match_info_free (match_info);
 
 done:
+    if (match_info)
+        g_match_info_free (match_info);
     if (r)
         g_regex_unref (r);
     mm_serial_port_close (MM_SERIAL_PORT (port));
diff --git a/plugins/mm-modem-novatel-gsm.c b/plugins/mm-modem-novatel-gsm.c
index 5d78db7..706664c 100644
--- a/plugins/mm-modem-novatel-gsm.c
+++ b/plugins/mm-modem-novatel-gsm.c
@@ -198,8 +198,6 @@ parse_nwrat_response (GString *response,
     mode = atoi (str);
     g_free (str);
 
-    g_match_info_free (match_info);
-
     if (mode < 0 || mode > 2) {
         g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL,
                              "Failed to parse mode/tech response");
@@ -219,6 +217,7 @@ parse_nwrat_response (GString *response,
     success = TRUE;
 
 out:
+    g_match_info_free (match_info);
     g_regex_unref (r);
     return success;
 }
diff --git a/plugins/mm-modem-samsung-gsm.c b/plugins/mm-modem-samsung-gsm.c
index df7a4b4..976d419 100755
--- a/plugins/mm-modem-samsung-gsm.c
+++ b/plugins/mm-modem-samsung-gsm.c
@@ -357,9 +357,10 @@ send_samsung_pinnum_done (MMAtSerialPort *port,
 
     mm_callback_info_set_result (info, GUINT_TO_POINTER (attempts_left), NULL);
 
-    g_match_info_free (match_info);
 
 done:
+    if (match_info)
+        g_match_info_free (match_info);
     if (r)
         g_regex_unref (r);
     mm_serial_port_close (MM_SERIAL_PORT (port));
diff --git a/plugins/mm-modem-sierra-gsm.c b/plugins/mm-modem-sierra-gsm.c
index 6d4e4d5..c15598f 100644
--- a/plugins/mm-modem-sierra-gsm.c
+++ b/plugins/mm-modem-sierra-gsm.c
@@ -72,7 +72,7 @@ get_allowed_mode_done (MMAtSerialPort *port,
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
     GRegex *r = NULL;
-    GMatchInfo *match_info;
+    GMatchInfo *match_info = NULL;
 
     /* If the modem has already been removed, return without
      * scheduling callback */
@@ -127,6 +127,8 @@ get_allowed_mode_done (MMAtSerialPort *port,
     }
 
 done:
+    if (match_info)
+        g_match_info_free (match_info);
     if (r)
         g_regex_unref (r);
     mm_callback_info_schedule (info);
diff --git a/plugins/mm-modem-x22x-gsm.c b/plugins/mm-modem-x22x-gsm.c
index 012733d..a31cd36 100644
--- a/plugins/mm-modem-x22x-gsm.c
+++ b/plugins/mm-modem-x22x-gsm.c
@@ -81,8 +81,6 @@ parse_syssel_response (GString *response,
     mode = atoi (str);
     g_free (str);
 
-    g_match_info_free (match_info);
-
     if (mode < 0 || mode > 2) {
         g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL,
                              "Failed to parse mode/tech response");
@@ -102,6 +100,7 @@ parse_syssel_response (GString *response,
     success = TRUE;
 
 out:
+    g_match_info_free (match_info);
     g_regex_unref (r);
     return success;
 }
diff --git a/plugins/mm-modem-zte.c b/plugins/mm-modem-zte.c
index 0f69328..09e5090 100644
--- a/plugins/mm-modem-zte.c
+++ b/plugins/mm-modem-zte.c
@@ -101,7 +101,7 @@ get_allowed_mode_done (MMAtSerialPort *port,
 {
     MMCallbackInfo *info = (MMCallbackInfo *) user_data;
     GRegex *r = NULL;
-    GMatchInfo *match_info;
+    GMatchInfo *match_info = NULL;
 
     /* If the modem has already been removed, return without
      * scheduling callback */
@@ -134,8 +134,6 @@ get_allowed_mode_done (MMAtSerialPort *port,
         pref_acq = atoi (str);
         g_free (str);
 
-        g_match_info_free (match_info);
-
         if (cm_mode < 0 || cm_mode > 2 || pref_acq < 0 || pref_acq > 2) {
             info->error = g_error_new (MM_MODEM_ERROR,
                                        MM_MODEM_ERROR_GENERAL,
@@ -160,6 +158,8 @@ get_allowed_mode_done (MMAtSerialPort *port,
     }
 
 done:
+    if (match_info)
+        g_match_info_free (match_info);
     if (r)
         g_regex_unref (r);
     mm_callback_info_schedule (info);
diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c
index 47fe063..44cfe23 100644
--- a/src/mm-generic-gsm.c
+++ b/src/mm-generic-gsm.c
@@ -2868,6 +2868,8 @@ handle_reg_status_response (MMGenericGsm *self,
         return FALSE;
     }
 
+    g_match_info_free (match_info);
+
     /* Success; update cached location information */
     update_lac_ci (self, lac, ci, cgreg ? 1 : 0);
 
@@ -3567,6 +3569,8 @@ cid_range_read (MMAtSerialPort *port,
             if (cid == 0)
                 /* Choose something */
                 cid = 1;
+            g_match_info_free (match_info);
+            g_regex_unref (r);
         }
     } else
         info->error = g_error_new_literal (MM_MODEM_ERROR,
diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index f6a0ffa..66c3fb6 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -125,10 +125,8 @@ mm_gsm_parse_scan_response (const char *reply, GError **error)
     /* If we didn't get any hits, try the pre-UMTS format match */
     if (!g_regex_match (r, reply, 0, &match_info)) {
         g_regex_unref (r);
-        if (match_info) {
-            g_match_info_free (match_info);
-            match_info = NULL;
-        }
+        g_match_info_free (match_info);
+        match_info = NULL;
 
         /* Pre-UMTS format doesn't include the cell access technology after
          * the numeric operator element.
@@ -778,8 +776,8 @@ mm_gsm_parse_cscs_support_response (const char *reply,
             g_match_info_next (match_info, NULL);
             success = TRUE;
         }
-        g_match_info_free (match_info);
     }
+    g_match_info_free (match_info);
     g_regex_unref (r);
 
     if (success)
@@ -1020,8 +1018,8 @@ mm_parse_cind_test_response (const char *reply, GError **error)
 
             g_match_info_next (match_info, NULL);
         }
-        g_match_info_free (match_info);
     }
+    g_match_info_free (match_info);
     g_regex_unref (r);
 
     return hash;
@@ -1086,11 +1084,10 @@ mm_parse_cind_query_response(const char *reply, GError **error)
         g_free (str);
         g_match_info_next (match_info, NULL);
     }
-    g_match_info_free (match_info);
 
 done:
-    if (r)
-        g_regex_unref (r);
+    g_match_info_free (match_info);
+    g_regex_unref (r);
 
     return array;
 }
diff --git a/src/mm-serial-parsers.c b/src/mm-serial-parsers.c
index 344e1bc..43c35f4 100644
--- a/src/mm-serial-parsers.c
+++ b/src/mm-serial-parsers.c
@@ -125,8 +125,6 @@ mm_serial_parser_v0_parse (gpointer data,
         } else
             code = MM_MOBILE_ERROR_UNKNOWN;
 
-        g_match_info_free (match_info);
-
         switch (code) {
         case 0: /* OK */
             break;
@@ -155,6 +153,8 @@ mm_serial_parser_v0_parse (gpointer data,
         remove_matches (parser->generic_response, response);
     }
 
+    g_match_info_free (match_info);
+
     if (!found) {
         found = g_regex_match_full (parser->detailed_error, response->str, response->len, 0, 0, &match_info, NULL);
 
@@ -166,9 +166,9 @@ mm_serial_parser_v0_parse (gpointer data,
             } else
                 code = MM_MOBILE_ERROR_UNKNOWN;
 
-            g_match_info_free (match_info);
             local_error = mm_mobile_error_for_code (code);
         }
+        g_match_info_free (match_info);
     }
 
     if (found)
@@ -260,7 +260,7 @@ mm_serial_parser_v1_parse (gpointer data,
     GMatchInfo *match_info;
     GError *local_error = NULL;
     gboolean found = FALSE;
-    char *str;
+    char *str = NULL;
     int code;
 
     g_return_val_if_fail (parser != NULL, FALSE);
@@ -306,10 +306,9 @@ mm_serial_parser_v1_parse (gpointer data,
             str = g_match_info_fetch (match_info, 1);
             g_assert (str);
             local_error = mm_mobile_error_for_code (atoi (str));
-            g_free (str);
-            g_match_info_free (match_info);
             goto done;
         }
+        g_match_info_free (match_info);
     }
 
     /* Numeric CME errors */
@@ -320,10 +319,9 @@ mm_serial_parser_v1_parse (gpointer data,
         str = g_match_info_fetch (match_info, 1);
         g_assert (str);
         local_error = mm_mobile_error_for_code (atoi (str));
-        g_free (str);
-        g_match_info_free (match_info);
         goto done;
     }
+    g_match_info_free (match_info);
 
     /* Numeric CMS errors */
     /* Todo
@@ -337,10 +335,9 @@ mm_serial_parser_v1_parse (gpointer data,
         str = g_match_info_fetch (match_info, 1);
         g_assert (str);
         local_error = mm_mobile_error_for_code (atoi (str));
-        g_free (str);
-        g_match_info_free (match_info);
         goto done;
     }
+    g_match_info_free (match_info);
 
     /* String CME errors */
     found = g_regex_match_full (parser->regex_cme_error_str,
@@ -350,10 +347,9 @@ mm_serial_parser_v1_parse (gpointer data,
         str = g_match_info_fetch (match_info, 1);
         g_assert (str);
         local_error = mm_mobile_error_for_string (str);
-        g_free (str);
-        g_match_info_free (match_info);
         goto done;
     }
+    g_match_info_free (match_info);
 
     /* Motorola EZX errors */
     found = g_regex_match_full (parser->regex_ezx_error,
@@ -363,19 +359,19 @@ mm_serial_parser_v1_parse (gpointer data,
         str = g_match_info_fetch (match_info, 1);
         g_assert (str);
         local_error = mm_mobile_error_for_code (MM_MOBILE_ERROR_UNKNOWN);
-        g_free (str);
-        g_match_info_free (match_info);
         goto done;
     }
+    g_match_info_free (match_info);
 
     /* Last resort; unknown error */
     found = g_regex_match_full (parser->regex_unknown_error,
                                 response->str, response->len,
-                                0, 0, NULL, NULL);
+                                0, 0, &match_info, NULL);
     if (found) {
         local_error = mm_mobile_error_for_code (MM_MOBILE_ERROR_UNKNOWN);
         goto done;
     }
+    g_match_info_free (match_info);
 
     /* Connection failures */
     found = g_regex_match_full (parser->regex_connect_failed,
@@ -398,13 +394,12 @@ mm_serial_parser_v1_parse (gpointer data,
             code = MM_MODEM_CONNECT_ERROR_NO_CARRIER;
         }
 
-        g_free (str);
-        g_match_info_free (match_info);
-
         local_error = mm_modem_connect_error_for_code (code);
     }
 
 done:
+    g_free (str);
+    g_match_info_free (match_info);
     if (found)
         response_clean (response);
 
-- 
1.7.3.1
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]