Re: [MM] [PATCH] novatel-lte: normalize QMI status when included in DBus error message

[Aleksander Morgado <aleksander lanedo com>]

On 26/05/13 08:43, Ben Chan wrote:
> This patches normalize a response for the AT$NWQMISTATUS command, by
> replacing white-space characters with a space, before the response is
> included in a DBus error message.
> ---


If the only purpose is to normalize the message when it's set in a
GError/GAsyncResult, why not perform the normalization *just before*
it's needed? That would save unneeded heap allocations and g_free()s
when the string is not going to be used.



>  plugins/novatel/mm-broadband-bearer-novatel-lte.c | 41 ++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
> b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
> index 31c5650..6329414 100644
> --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
> +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
> @@ -42,6 +42,22 @@ struct _MMBroadbandBearerNovatelLtePrivate {
>      guint connection_poller;
>  };
>  
> +static gchar *
> +normalize_qmistatus (const gchar *status)
> +{
> +    gchar *normalized_status, *iter;
> +
> +    if (!status)
> +        return NULL;
> +
> +    normalized_status = g_strdup (status);
> +    for (iter = normalized_status; *iter; iter++)
> +        if (g_ascii_isspace (*iter))
> +            *iter = ' ';
> +
> +    return normalized_status;
> +}
> +
>  /*****************************************************************************/
>  /* 3GPP Connection sequence */
>  
> @@ -145,12 +161,12 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
>                                GAsyncResult *res,
>                                DetailedConnectContext *ctx)
>  {
> -    const gchar *result;
> +    gchar *result;
>      GError *error = NULL;
>  
> -    result = mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx->modem),
> -                                              res,
> -                                              &error);
> +    result = normalize_qmistatus (mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx->modem),
> +                                                                   res,
> +                                                                   &error));
>      if (!result) {
>          mm_warn ("QMI connection status failed: %s", error->message);
>          if (!g_error_matches (error, MM_MOBILE_EQUIPMENT_ERROR, MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN)) {
> @@ -159,7 +175,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
>              return;
>          }
>          g_error_free (error);
> -        result = "Unknown error";
> +        result = g_strdup ("Unknown error");
>      } else if (is_qmistatus_connected (result)) {
>          MMBearerIpConfig *config;
>  
> @@ -175,6 +191,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
>              (GDestroyNotify)mm_bearer_connect_result_unref);
>          g_object_unref (config);
>          detailed_connect_context_complete_and_free (ctx);
> +        g_free (result);
>          return;
>      }
>  
> @@ -184,6 +201,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
>          mm_dbg ("Retrying status check in a second. %d retries left.",
>                  ctx->retries);
>          g_timeout_add_seconds (1, (GSourceFunc)connect_3gpp_qmistatus, ctx);
> +        g_free (result);
>          return;
>      }
>  
> @@ -193,6 +211,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
>                                       MM_CORE_ERROR_FAILED,
>                                       "%s", result);
>      detailed_connect_context_complete_and_free (ctx);
> +    g_free (result);
>  }
>  
>  static gboolean
> @@ -366,18 +385,19 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
>                                GAsyncResult *res,
>                                DetailedDisconnectContext *ctx)
>  {
> -    const gchar *result;
> +    gchar *result;
>      GError *error = NULL;
>      gboolean is_connected = FALSE;
>  
> -    result = mm_base_modem_at_command_finish (MM_BASE_MODEM (modem),
> -                                              res,
> -                                              &error);
> +    result = normalize_qmistatus (mm_base_modem_at_command_finish (MM_BASE_MODEM (modem),
> +                                                                   res,
> +                                                                   &error));
>      if (result) {
>          mm_dbg ("QMI connection status: %s", result);
>          if (is_qmistatus_disconnected (result)) {
>              g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>              detailed_disconnect_context_complete_and_free (ctx);
> +            g_free (result);
>              return;
>          } else if (is_qmistatus_connected (result)) {
>              is_connected = TRUE;
> @@ -385,6 +405,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
>      } else {
>          mm_dbg ("QMI connection status failed: %s", error->message);
>          g_error_free (error);
> +        result = g_strdup ("Unknown error");
>      }
>  
>      if (ctx->retries > 0) {
> @@ -392,6 +413,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
>          mm_dbg ("Retrying status check in a second. %d retries left.",
>                  ctx->retries);
>          g_timeout_add_seconds (1, (GSourceFunc)disconnect_3gpp_qmistatus, ctx);
> +        g_free (result);
>          return;
>      }
>  
> @@ -408,6 +430,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
>          g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>  
>      detailed_disconnect_context_complete_and_free (ctx);
> +    g_free (result);
>  }
>  
>  static gboolean


-- 
Aleksander