[PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

[Dan Williams]

On Sat, 2017-03-25 at 21:39 +0100, Aleksander Morgado wrote:
> The method doing the operator name normalization takes as input the
> current configured modem charset. If this is UCS2, we will now just
> assume this is a hint: the string may or may not come in hex/UCS2.
> 
> This logic makes the custom operator name loading in Huawei unneeded,
> if the modem is configured in UCS2, we still properly process
> operator
> names coming in plain ASCII.

LGTM.

Dan

> ---
>  plugins/huawei/mm-broadband-modem-huawei.c | 51 --------------------
> ----------
>  src/mm-modem-helpers.c                     | 26 +++++++++------
>  src/tests/test-modem-helpers.c             | 45
> ++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+), 60 deletions(-)
> 
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c
> b/plugins/huawei/mm-broadband-modem-huawei.c
> index 74e680b4..9f13b0f2 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -2172,55 +2172,6 @@ modem_3gpp_disable_unsolicited_events
> (MMIfaceModem3gpp *self,
>  }
>  
>  /*******************************************************************
> **********/
> -/* Operator Name loading (3GPP interface) */
> -
> -static gchar *
> -modem_3gpp_load_operator_name_finish (MMIfaceModem3gpp *self,
> -                                      GAsyncResult *res,
> -                                      GError **error)
> -{
> -    const gchar *result;
> -    gchar *operator_name = NULL;
> -
> -    result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self),
> res, error);
> -    if (!result)
> -        return NULL;
> -
> -    if (!mm_3gpp_parse_cops_read_response (result,
> -                                           NULL, /* mode */
> -                                           NULL, /* format */
> -                                           &operator_name,
> -                                           NULL, /* act */
> -                                           error))
> -        return NULL;
> -
> -    /* Despite +CSCS? may claim supporting UCS2, Huawei modems
> always report the
> -     * operator name in ASCII in a +COPS response. Thus, we ignore
> the current
> -     * charset claimed by the modem and assume the charset is IRA
> when parsing
> -     * the operator name.
> -     */
> -    mm_3gpp_normalize_operator_name (&operator_name,
> MM_MODEM_CHARSET_IRA);
> -    if (operator_name)
> -        mm_dbg ("loaded Operator Name: %s", operator_name);
> -
> -    return operator_name;
> -}
> -
> -static void
> -modem_3gpp_load_operator_name (MMIfaceModem3gpp *self,
> -                               GAsyncReadyCallback callback,
> -                               gpointer user_data)
> -{
> -    mm_dbg ("loading Operator Name (huawei)...");
> -    mm_base_modem_at_command (MM_BASE_MODEM (self),
> -                              "+COPS=3,0;+COPS?",
> -                              3,
> -                              FALSE,
> -                              callback,
> -                              user_data);
> -}
> -
> -/*******************************************************************
> **********/
>  /* Create Bearer (Modem interface) */
>  
>  typedef struct {
> @@ -4603,8 +4554,6 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface)
>      iface->enable_unsolicited_events_finish =
> modem_3gpp_enable_unsolicited_events_finish;
>      iface->disable_unsolicited_events =
> modem_3gpp_disable_unsolicited_events;
>      iface->disable_unsolicited_events_finish =
> modem_3gpp_disable_unsolicited_events_finish;
> -    iface->load_operator_name = modem_3gpp_load_operator_name;
> -    iface->load_operator_name_finish =
> modem_3gpp_load_operator_name_finish;
>  }
>  
>  static void
> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
> index fc95e28f..9266a5a0 100644
> --- a/src/mm-modem-helpers.c
> +++ b/src/mm-modem-helpers.c
> @@ -3131,20 +3131,28 @@ mm_3gpp_normalize_operator_name
> (gchar          **operator,
>      if (*operator == NULL)
>          return;
>  
> -    /* Some modems (Option & HSO) return the operator name as a
> hexadecimal
> -     * string of the bytes of the operator name as encoded by the
> current
> -     * character set.
> -     */
> +    /* Despite +CSCS? may claim supporting UCS2, Some modems (e.g.
> Huawei)
> +     * always report the operator name in ASCII in a +COPS response.
> */
>      if (cur_charset == MM_MODEM_CHARSET_UCS2) {
> +        gchar *tmp;
> +
> +        tmp = g_strdup (*operator);
>          /* In this case we're already checking UTF-8 validity */
> -        *operator = mm_charset_take_and_convert_to_utf8 (*operator,
> MM_MODEM_CHARSET_UCS2);
> +        tmp = mm_charset_take_and_convert_to_utf8 (tmp,
> cur_charset);
> +        if (tmp) {
> +            g_clear_pointer (operator, g_free);
> +            *operator = tmp;
> +            goto out;
> +        }
>      }
> -    /* Ensure the operator name is valid UTF-8 so that we can send
> it
> -     * through D-Bus and such.
> -     */
> -    else if (!g_utf8_validate (*operator, -1, NULL))
> +
> +    /* Charset is unknown or there was an error in conversion; try
> to see
> +     * if the contents we got are valid UTF-8 already. */
> +    if (!g_utf8_validate (*operator, -1, NULL))
>          g_clear_pointer (operator, g_free);
>  
> +out:
> +
>      /* Some modems (Novatel LTE) return the operator name as
> "Unknown" when
>       * it fails to obtain the operator name. Return NULL in such
> case.
>       */
> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-
> helpers.c
> index 1fc8c353..98f30f83 100644
> --- a/src/tests/test-modem-helpers.c
> +++ b/src/tests/test-modem-helpers.c
> @@ -866,6 +866,49 @@ test_cops_query (void)
>  }
>  
>  /*******************************************************************
> **********/
> +
> +typedef struct {
> +    const gchar    *input;
> +    MMModemCharset  charset;
> +    const gchar    *normalized;
> +} NormalizeOperatorTest;
> +
> +static const NormalizeOperatorTest normalize_operator_tests[] = {
> +    /* charset unknown */
> +    { "Verizon", MM_MODEM_CHARSET_UNKNOWN, "Verizon" },
> +    /* charset configured as IRA (ASCII) */
> +    { "Verizon", MM_MODEM_CHARSET_IRA, "Verizon" },
> +    /* charset configured as GSM7 */
> +    { "Verizon", MM_MODEM_CHARSET_GSM, "Verizon" },
> +    /* charset configured as UCS2 */
> +    { "0056006500720069007A006F006E", MM_MODEM_CHARSET_UCS2,
> "Verizon" },
> +    { "Verizon",                      MM_MODEM_CHARSET_UCS2,
> "Verizon" },
> +};
> +
> +static void
> +common_test_normalize_operator (const NormalizeOperatorTest *t)
> +{
> +    gchar *str;
> +
> +    str = g_strdup (t->input);
> +    mm_3gpp_normalize_operator_name (&str, t->charset);
> +    if (!t->normalized)
> +        g_assert (!str);
> +    else
> +        g_assert_cmpstr (str, ==, t->normalized);
> +    g_free (str);
> +}
> +
> +static void
> +test_normalize_operator (void)
> +{
> +    guint i;
> +
> +    for (i = 0; i < G_N_ELEMENTS (normalize_operator_tests); i++)
> +        common_test_normalize_operator
> (&normalize_operator_tests[i]);
> +}
> +
> +/*******************************************************************
> **********/
>  /* Test CREG/CGREG responses and unsolicited messages */
>  
>  typedef struct {
> @@ -3527,6 +3570,8 @@ int main (int argc, char **argv)
>  
>      g_test_suite_add (suite, TESTCASE (test_cops_query, NULL));
>  
> +    g_test_suite_add (suite, TESTCASE (test_normalize_operator,
> NULL));
> +
>      g_test_suite_add (suite, TESTCASE (test_creg1_solicited,
> reg_data));
>      g_test_suite_add (suite, TESTCASE (test_creg1_unsolicited,
> reg_data));
>      g_test_suite_add (suite, TESTCASE (test_creg2_mercury_solicited,
> reg_data));