Re: [PATCH 4/9] Make sim operations return sim error codes to core.

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Andrew,

On 06/29/2010 05:14 AM, Andrzej Zaborowski wrote:
> ---
>  drivers/atmodem/stk.c |   20 ++++++++++++--------
>  include/types.h       |    1 +

When you're touching more than two directories, always break the patches
up / directory.  In this case resubmit two patches, one for types.h and
separate patch for stk.c

>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
> index 8cff4a2..aede668 100644
> --- a/drivers/atmodem/stk.c
> +++ b/drivers/atmodem/stk.c
> @@ -74,11 +74,13 @@ static void at_csim_envelope_cb(gboolean ok, GAtResult *result,
>  	if (rlen != len * 2 || len < 2)
>  		goto error;
>  
> -	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
> -		goto error;
> +	if ((response[len - 2] != 0x90 && response[len - 2] != 0x91) ||
> +			(response[len - 2] == 0x90 && response[len - 1] != 0)) {
> +		memset(&error, 0, sizeof(error));
>  
> -	if (response[len - 2] == 0x90 && response[len - 1] != 0)
> -		goto error;
> +		error.type = OFONO_ERROR_TYPE_SIM;
> +		error.error = (response[len - 2] << 8) | response[len - 1];
> +	}
>  
>  	DBG("csim_envelope_cb: %i", len);
>  
> @@ -157,11 +159,13 @@ static void at_csim_terminal_response_cb(gboolean ok, GAtResult *result,
>  	if (rlen != len * 2 || len < 2)
>  		goto error;
>  
> -	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
> -		goto error;
> +	if ((response[len - 2] != 0x90 && response[len - 2] != 0x91) ||
> +			(response[len - 2] == 0x90 && response[len - 1] != 0)) {
> +		memset(&error, 0, sizeof(error));
>  
> -	if (response[len - 2] == 0x90 && response[len - 1] != 0)
> -		goto error;
> +		error.type = OFONO_ERROR_TYPE_SIM;
> +		error.error = (response[len - 2] << 8) | response[len - 1];
> +	}
>  
>  	DBG("csim_terminal_response_cb: %i", len);
>  

Do you think it is a good idea to do this for all SIM elementary file
operations as well?

Regards,
-Denis