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

2013-05-27 Thread Aleksander Morgado 
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

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

2013-05-25 Thread Ben Chan 
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.
---
 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 (c