[1/1] atmodem: sim: when reading sim files, avoid incomplete result lines

Commit Message

Christophe Ronco March 28, 2024, 8:11 a.m. UTC

Modem ME310G1 sometimes add incomplete result lines to first AT+CRSM
command. Example:
AT+CRSM=192,12258
+CRSM: 0
+CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110

OK

Parse all result lines starting with prefix until a line with at least sw1
and sw2 parameters is found.
---
 drivers/atmodem/sim.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Denis Kenzior March 28, 2024, 3:17 p.m. UTC | #1

Hi Christophe,

On 3/28/24 03:11, Christophe Ronco wrote:
> Modem ME310G1 sometimes add incomplete result lines to first AT+CRSM
> command. Example:
> AT+CRSM=192,12258
> +CRSM: 0
> +CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110
> 
> OK
> 
> Parse all result lines starting with prefix until a line with at least sw1
> and sw2 parameters is found.
> ---
>   drivers/atmodem/sim.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 

<snip>

> @@ -117,11 +118,14 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
>   
>   	g_at_result_iter_init(&iter, result);
>   
> -	if (!g_at_result_iter_next(&iter, prefix))
> -		goto error;
> +	while (!nb_found) {
> +		if (!g_at_result_iter_next(&iter, prefix))
> +			goto error;
>   
> -	g_at_result_iter_next_number(&iter, &sw1);
> -	g_at_result_iter_next_number(&iter, &sw2);
> +		nb_found = g_at_result_iter_next_number(&iter, &sw1);
> +		if (nb_found)
> +			nb_found = g_at_result_iter_next_number(&iter, &sw2);
> +	}

I think you're going to need a comment in the code, there's no way anyone will 
understand what is happening here after a year or two.

Also consider a few other alternatives:

if (!g_at_result_iter_next(&iter, prefix))
	goto error;

/* Skip to last response, include comment as to why */
while (g_at_result_iter_next(&iter, prefix))
	;

... proceed as normal

Alternatively
const guint8 *response = NULL;

...

while (g_at_result_iter_next(&iter, prefix)) {
	if (!g_at_result_iter_next_number(&iter, &sw1))
		continue;

	/* validate sw1, can be 0x9* or 0x6* */
	if (!valid_sw1(...))
		continue;

	if (!g_at_result_iter_next(&iter, &sw2))
		continue;

	if (!g_at_result_iter_next_hexstring....) {
		cb(...)
		return;
	}
}

if (!response)
	goto error;

/* parse response */

>   
>   	if (!g_at_result_iter_next_hexstring(&iter, &response, &len) ||
>   			(sw1 != 0x90 && sw1 != 0x91 && sw1 != 0x92) ||

Regards,
-Denis

Patch

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index d75a09c2..46f21758 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -107,6 +107,7 @@  static void get_response_common_cb(gboolean ok, GAtResult *result,
 	int str;
 	unsigned char access[3];
 	unsigned char file_status;
+	gboolean nb_found = FALSE;
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
@@ -117,11 +118,14 @@  static void get_response_common_cb(gboolean ok, GAtResult *result,
 
 	g_at_result_iter_init(&iter, result);
 
-	if (!g_at_result_iter_next(&iter, prefix))
-		goto error;
+	while (!nb_found) {
+		if (!g_at_result_iter_next(&iter, prefix))
+			goto error;
 
-	g_at_result_iter_next_number(&iter, &sw1);
-	g_at_result_iter_next_number(&iter, &sw2);
+		nb_found = g_at_result_iter_next_number(&iter, &sw1);
+		if (nb_found)
+			nb_found = g_at_result_iter_next_number(&iter, &sw2);
+	}
 
 	if (!g_at_result_iter_next_hexstring(&iter, &response, &len) ||
 			(sw1 != 0x90 && sw1 != 0x91 && sw1 != 0x92) ||