[PATCHv2] sim: Use quoted string with AT+CRSM data parameter

[PATCHv2] sim: Use quoted string with AT+CRSM data parameter

[Nicolas Bertrand]

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

Fix issue with some modems preventing to update elementary files
(speedup, ZTE, huawei, MBM)
---
 drivers/atmodem/sim.c |  127 ++++++++++++++++---------------------------------
 1 files changed, 41 insertions(+), 86 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 8ee9822..a75d51f 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -298,71 +298,44 @@ static void at_crsm_update_cb(gboolean ok, GAtResult *result,
 	cb(&error, cbd->data);
 }
 
-static void at_sim_update_binary(struct ofono_sim *sim, int fileid,
-					int start, int length,
-					const unsigned char *value,
-					ofono_sim_write_cb_t cb, void *data)
-{
-	struct sim_data *sd = ofono_sim_get_data(sim);
-	struct cb_data *cbd = cb_data_new(cb, data);
-	char *buf = g_try_new(char, 36 + length * 2);
-	int len, ret;
-
-	if (buf == NULL)
-		goto error;
-
-	len = sprintf(buf, "AT+CRSM=214,%i,%i,%i,%i,", fileid,
-			start >> 8, start & 0xff, length);
-
-	for (; length; length--)
-		len += sprintf(buf + len, "%02hhX", *value++);
-
-	ret = g_at_chat_send(sd->chat, buf, crsm_prefix,
-				at_crsm_update_cb, cbd, g_free);
-
-	g_free(buf);
-
-	if (ret > 0)
-		return;
-
-error:
-	g_free(cbd);
-
-	CALLBACK_WITH_FAILURE(cb, data);
-}
-
-static void at_sim_update_record(struct ofono_sim *sim, int fileid,
-					int record, int length,
-					const unsigned char *value,
-					ofono_sim_write_cb_t cb, void *data)
+static void at_sim_update_file(struct ofono_sim *sim, int cmd, int fileid,
+		int p1, int p2, int p3, const unsigned char *value,
+		ofono_sim_write_cb_t cb, void *data)
 {
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char *buf;
+	char *quote = "";
 	int len, ret;
-	int size = 36 + length * 2;
+	int size = 36 + p3 * 2;
 
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		size += 2; /*Add quotes*/
+	DBG("");
+
+	/* Add quotes */
+	switch (sd->vendor) {
+	case OFONO_VENDOR_MBM:
+	case OFONO_VENDOR_ZTE:
+	case OFONO_VENDOR_HUAWEI:
+	case OFONO_VENDOR_SPEEDUP:
+		quote = "\"";
+		size += 2;
+		break;
+	}
 
 	buf = g_try_new(char, size);
 	if (buf == NULL)
 		goto error;
 
-	len = sprintf(buf, "AT+CRSM=220,%i,%i,4,%i,", fileid,
-			record, length);
+	len = sprintf(buf, "AT+CRSM=%i,%i,%i,%i,%i,%s", cmd, fileid,
+			p1, p2, p3, quote);
 
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		len += sprintf(buf + len, "\"");
-
-	for (; length; length--)
+	for (; p3; p3--)
 		len += sprintf(buf + len, "%02hhX", *value++);
 
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		sprintf(buf + len, "\"");
+	sprintf(buf + len, "%s", quote);
 
 	ret = g_at_chat_send(sd->chat, buf, crsm_prefix,
-				at_crsm_update_cb, cbd, g_free);
+			at_crsm_update_cb, cbd, g_free);
 
 	g_free(buf);
 
@@ -375,46 +348,28 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
-static void at_sim_update_cyclic(struct ofono_sim *sim, int fileid,
-					int length, const unsigned char *value,
-					ofono_sim_write_cb_t cb, void *data)
+static void at_sim_update_binary(struct ofono_sim *sim, int fileid,
+		int start, int length, const unsigned char *value,
+		ofono_sim_write_cb_t cb, void *data)
 {
-	struct sim_data *sd = ofono_sim_get_data(sim);
-	struct cb_data *cbd = cb_data_new(cb, data);
-	char *buf;
-	int len, ret;
-	int size = 36 + length * 2;
-
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		size += 2; /* Add quotes */
-
-	buf = g_try_new(char, size);
-	if (buf == NULL)
-		goto error;
-
-	len = sprintf(buf, "AT+CRSM=220,%i,0,3,%i,", fileid, length);
-
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		len += sprintf(buf + len, "\"");
-
-	for (; length; length--)
-		len += sprintf(buf + len, "%02hhX", *value++);
-
-	if (sd->vendor == OFONO_VENDOR_MBM)
-		sprintf(buf + len, "\"");
-
-	ret = g_at_chat_send(sd->chat, buf, crsm_prefix,
-				at_crsm_update_cb, cbd, g_free);
-
-	g_free(buf);
-
-	if (ret > 0)
-		return;
+	at_sim_update_file(sim, 214, fileid, start >> 8, start & 0xff,
+			length, value, cb, data);
+}
 
-error:
-	g_free(cbd);
+static void at_sim_update_record(struct ofono_sim *sim, int fileid,
+		int record, int length, const unsigned char *value,
+		ofono_sim_write_cb_t cb, void *data)
+{
+	at_sim_update_file(sim, 220, fileid, record, 4,
+			length, value, cb, data);
+}
 
-	CALLBACK_WITH_FAILURE(cb, data);
+static void at_sim_update_cyclic(struct ofono_sim *sim, int fileid,
+		int length, const unsigned char *value,
+		ofono_sim_write_cb_t cb, void *data)
+{
+	at_sim_update_file(sim, 220, fileid, 0, 3,
+			length, value, cb, data);
 }
 
 static void at_cimi_cb(gboolean ok, GAtResult *result, gpointer user_data)
-- 
1.7.5.4

Re: [PATCHv2] sim: Use quoted string with AT+CRSM data parameter

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Hi Nicolas,

On 03/22/2012 12:28 PM, Nicolas Bertrand wrote:
> Fix issue with some modems preventing to update elementary files
> (speedup, ZTE, huawei, MBM)
> ---
>  drivers/atmodem/sim.c |  127 ++++++++++++++++---------------------------------
>  1 files changed, 41 insertions(+), 86 deletions(-)
>

Patch has been applied, but I had to fix up numerous coding style
violations afterwards.  Please re-read doc/coding-style.txt and pay
extra attention next time.

Just one more comment:

> +	sprintf(buf + len, "%s", quote);
>  

Something like this is really wasteful, you're invoking sprintf (which
is quite a heavy hammer) just to put in a quote character.  Use buf[len]
= '\"' next time.

Regards,
-Denis