colin/ModemManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cinterion: make sure FALSE sets GError in parse_smong_response()

Browse Source 
The g_regex_match_full() method may return FALSE without setting the
GError, so that case needs to be considered.

In addition to that, the following g_assert() was not doing what it
should have been, as it was comparing the address of the variable and
not the variable itself; rework the code to avoid that as well:
  g_assert (access_tech != MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN);
This commit is contained in:
Aleksander Morgado
2021-03-21 13:44:16 +01:00
parent d01bca493d
commit 634682b602
2 changed files with 48 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
plugins/cinterion/mm-modem-helpers-cinterion.c
View File

@@ -940,19 +940,23 @@ mm_cinterion_parse_sgauth_response (const gchar *response,
/*****************************************************************************/ /*****************************************************************************/
/* ^SMONG response parser */ /* ^SMONG response parser */
static MMModemAccessTechnology static gboolean
get_access_technology_from_smong_gprs_status (guint gprs_status, get_access_technology_from_smong_gprs_status (guint gprs_status,
GError **error) MMModemAccessTechnology *out,
GError **error)
{ {
switch (gprs_status) { switch (gprs_status) {
case 0: case 0:
return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; *out = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
return TRUE;
case 1: case 1:
case 2: case 2:
return MM_MODEM_ACCESS_TECHNOLOGY_GPRS; *out = MM_MODEM_ACCESS_TECHNOLOGY_GPRS;
return TRUE;
case 3: case 3:
case 4: case 4:
return MM_MODEM_ACCESS_TECHNOLOGY_EDGE; *out = MM_MODEM_ACCESS_TECHNOLOGY_EDGE;
return TRUE;
default: default:
break; break;
} }
@@ -963,7 +967,7 @@ get_access_technology_from_smong_gprs_status (guint gprs_status,
"Couldn't get network capabilities, " "Couldn't get network capabilities, "
"unsupported GPRS status value: '%u'", "unsupported GPRS status value: '%u'",
gprs_status); gprs_status);
return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN; return FALSE;
} }
gboolean gboolean
@@ -971,9 +975,10 @@ mm_cinterion_parse_smong_response (const gchar *response,
MMModemAccessTechnology *access_tech, MMModemAccessTechnology *access_tech,
GError **error) GError **error)
{ {
GError *inner_error = NULL; guint value = 0;
GMatchInfo *match_info = NULL; GError *inner_error = NULL;
GRegex *regex; g_autoptr(GMatchInfo) match_info = NULL;
g_autoptr(GRegex) regex;
/* The AT^SMONG command returns a cell info table, where the second /* The AT^SMONG command returns a cell info table, where the second
* column identifies the "GPRS status", which is exactly what we want. * column identifies the "GPRS status", which is exactly what we want.
@@ -992,26 +997,21 @@ mm_cinterion_parse_smong_response (const gchar *response,
0, NULL); 0, NULL);
g_assert (regex); g_assert (regex);
if (g_regex_match_full (regex, response, strlen (response), 0, 0, &match_info, &inner_error)) { g_regex_match_full (regex, response, strlen (response), 0, 0, &match_info, &inner_error);
guint value = 0;
if (!mm_get_uint_from_match_info (match_info, 2, &value))
inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
"Couldn't read 'GPRS status' field from AT^SMONG response");
else if (access_tech)
*access_tech = get_access_technology_from_smong_gprs_status (value, &inner_error);
}
g_match_info_free (match_info);
g_regex_unref (regex);
if (inner_error) { if (inner_error) {
g_prefix_error (&inner_error, "Failed to match AT^SMONG response: ");
g_propagate_error (error, inner_error); g_propagate_error (error, inner_error);
return FALSE; return FALSE;
} }
g_assert (access_tech != MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN); if (!g_match_info_matches (match_info) || !mm_get_uint_from_match_info (match_info, 2, &value)) {
return TRUE; g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
"Couldn't read 'GPRS status' field from AT^SMONG response");
return FALSE;
}
return get_access_technology_from_smong_gprs_status (value, access_tech, error);
} }
/*****************************************************************************/ /*****************************************************************************/

29
plugins/cinterion/tests/test-modem-helpers-cinterion.c
View File

@@ -1134,6 +1134,7 @@ test_sind_response_simstatus (void)
static void static void
common_test_smong_response (const gchar *response, common_test_smong_response (const gchar *response,
gboolean success,
MMModemAccessTechnology expected_access_tech) MMModemAccessTechnology expected_access_tech)
{ {
GError *error = NULL; GError *error = NULL;
@@ -1141,10 +1142,15 @@ common_test_smong_response (const gchar *response,
MMModemAccessTechnology access_tech; MMModemAccessTechnology access_tech;
res = mm_cinterion_parse_smong_response (response, &access_tech, &error); res = mm_cinterion_parse_smong_response (response, &access_tech, &error);
g_assert_no_error (error);
g_assert (res == TRUE);
g_assert_cmpuint (access_tech, ==, expected_access_tech); if (success) {
g_assert_no_error (error);
g_assert (res);
g_assert_cmpuint (access_tech, ==, expected_access_tech);
} else {
g_assert (error);
g_assert (!res);
}
} }
static void static void
@@ -1155,7 +1161,7 @@ test_smong_response_tc63i (void)
"GPRS Monitor\r\n" "GPRS Monitor\r\n"
"BCCH G PBCCH PAT MCC MNC NOM TA RAC # Cell #\r\n" "BCCH G PBCCH PAT MCC MNC NOM TA RAC # Cell #\r\n"
"0073 1 - - 262 02 2 00 01\r\n"; "0073 1 - - 262 02 2 00 01\r\n";
common_test_smong_response (response, MM_MODEM_ACCESS_TECHNOLOGY_GPRS); common_test_smong_response (response, TRUE, MM_MODEM_ACCESS_TECHNOLOGY_GPRS);
} }
static void static void
@@ -1167,7 +1173,19 @@ test_smong_response_other (void)
"\r\n" "\r\n"
"BCCH G PBCCH PAT MCC MNC NOM TA RAC # Cell #\r\n" "BCCH G PBCCH PAT MCC MNC NOM TA RAC # Cell #\r\n"
" 44 1 - - 234 10 - - - \r\n"; " 44 1 - - 234 10 - - - \r\n";
common_test_smong_response (response, MM_MODEM_ACCESS_TECHNOLOGY_GPRS); common_test_smong_response (response, TRUE, MM_MODEM_ACCESS_TECHNOLOGY_GPRS);
}
static void
test_smong_response_no_match (void)
{
const gchar *response =
"\r\n"
"GPRS Monitor\r\n"
"\r\n"
"BCCH K PBCCH PAT MCC MNC NOM TA RAC # Cell #\r\n"
" 44 1 - - 234 10 - - - \r\n";
common_test_smong_response (response, FALSE, MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN);
} }
/*****************************************************************************/ /*****************************************************************************/
@@ -1757,6 +1775,7 @@ int main (int argc, char **argv)
g_test_add_func ("/MM/cinterion/sind/response/simstatus", test_sind_response_simstatus); g_test_add_func ("/MM/cinterion/sind/response/simstatus", test_sind_response_simstatus);
g_test_add_func ("/MM/cinterion/smong/response/tc63i", test_smong_response_tc63i); g_test_add_func ("/MM/cinterion/smong/response/tc63i", test_smong_response_tc63i);
g_test_add_func ("/MM/cinterion/smong/response/other", test_smong_response_other); g_test_add_func ("/MM/cinterion/smong/response/other", test_smong_response_other);
g_test_add_func ("/MM/cinterion/smong/response/no-match", test_smong_response_no_match);
g_test_add_func ("/MM/cinterion/slcc/urc/empty", test_slcc_urc_empty); g_test_add_func ("/MM/cinterion/slcc/urc/empty", test_slcc_urc_empty);
g_test_add_func ("/MM/cinterion/slcc/urc/single", test_slcc_urc_single); g_test_add_func ("/MM/cinterion/slcc/urc/single", test_slcc_urc_single);
g_test_add_func ("/MM/cinterion/slcc/urc/multiple", test_slcc_urc_multiple); g_test_add_func ("/MM/cinterion/slcc/urc/multiple", test_slcc_urc_multiple);