Re: [PATCH] Fix: simplify isimodem call-barring driver.

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Pekka,

> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Add support for lock codes AG and AC (MMI codes 333 and 353. respectively).

First of all, please keep your commit messages less than 72 characters long 
and headings less than 50 characters long.

Secondly, please keep your patches whitespace clean, there are several cases 
of spaces being used instead of tabs for indentation.

> ---
>  drivers/isimodem/call-barring.c |  153
>  ++++++++++++--------------------------- drivers/isimodem/ss.h           | 
>   28 ++++----
>  2 files changed, 63 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/isimodem/call-barring.c
>  b/drivers/isimodem/call-barring.c index 6487ae8..b0246fa 100644
> --- a/drivers/isimodem/call-barring.c
> +++ b/drivers/isimodem/call-barring.c
> @@ -48,6 +48,28 @@ struct barr_data {
>  	GIsiClient *client;
>  };

> 
> +	DBG("lock code %s (%u) old password %s new password %s\n",
> +	    lock, ss_code, old_passwd, new_passwd);

Here is an instance of whitespace error.

> -	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
> +	if (cbd &&
> +	    g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
>  				set_passwd_resp_cb, cbd))
>  		return;

Here's another one..

>  enum ss_codes {
> -	SS_GSM_ALL_FORWARDINGS = 0x02,
> -	SS_GSM_ALL_COND_FORWARDINGS = 0x04,
> -	SS_GSM_FORW_UNCONDITIONAL = 0x15,
> -	SS_GSM_BARR_ALL_OUT = 0x21,
> -	SS_GSM_BARR_ALL_IN = 0x23,
> -	SS_GSM_CALL_WAITING = 0x2B,
> -	SS_GSM_FORW_NO_REPLY = 0x3D,
> -	SS_GSM_FORW_NO_REACH = 0x3E,
> -	SS_GSM_FORW_BUSY = 0x43,
> -	SS_GSM_ALL_BARRINGS = 0x014A,
> -	SS_GSM_BARR_OUT_INTER = 0x014B,
> -	SS_GSM_BARR_OUT_INTER_EXC_HOME = 0x014C,
> -	SS_GSM_BARR_ALL_IN_ROAM = 0x015F
> +	SS_GSM_ALL_FORWARDINGS = 002,
> +	SS_GSM_ALL_COND_FORWARDINGS = 004,

Are you sure this isn't octal syntax?  It still works, but you might simply 
use 2 and 4  here.

> +	SS_GSM_FORW_UNCONDITIONAL = 21,
> +	SS_GSM_BARR_ALL_OUT = 33,
> +	SS_GSM_OUTGOING_BARR_SERV = 333,
> +	SS_GSM_INCOMING_BARR_SERV = 353,
> +	SS_GSM_BARR_ALL_IN = 35,
> +	SS_GSM_CALL_WAITING = 43,
> +	SS_GSM_FORW_NO_REPLY = 61,
> +	SS_GSM_FORW_NO_REACH = 62,
> +	SS_GSM_FORW_BUSY = 67,
> +	SS_GSM_ALL_BARRINGS = 330,
> +	SS_GSM_BARR_OUT_INTER = 331,
> +	SS_GSM_BARR_OUT_INTER_EXC_HOME = 332,
> +	SS_GSM_BARR_ALL_IN_ROAM = 351,
>  };

From now on we'll require all enum values to be tab aligned so they're a bit 
easier to read.  See src/stkutil.h for reference.

Regards,
-Denis