[PATCH 1/7] Barring services can only be activated/deactivated.

[PATCH 1/7] Barring services can only be activated/deactivated.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 src/call-barring.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/call-barring.c b/src/call-barring.c
index 2a04b25..13d625e 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -457,13 +457,13 @@ static gboolean cb_ss_control(int type, const char *sc,
 	switch (type) {
 	case SS_CONTROL_TYPE_ACTIVATION:
 	case SS_CONTROL_TYPE_REGISTRATION:
-		cb->ss_req_type = SS_CONTROL_TYPE_REGISTRATION;
+		cb->ss_req_type = SS_CONTROL_TYPE_ACTIVATION;
 		cb->driver->set(cb, fac, 1, sia, cls,
 				cb_ss_set_lock_callback, cb);
 		break;
 	case SS_CONTROL_TYPE_ERASURE:
 	case SS_CONTROL_TYPE_DEACTIVATION:
-		cb->ss_req_type = SS_CONTROL_TYPE_ERASURE;
+		cb->ss_req_type = SS_CONTROL_TYPE_DEACTIVATION;
 		cb->driver->set(cb, fac, 0, sia, cls,
 				cb_ss_set_lock_callback, cb);
 		break;
-- 
1.6.3.3

[PATCH 2/7] Return newly-queried barring status.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 src/call-barring.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/call-barring.c b/src/call-barring.c
index 13d625e..530d64b 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -148,7 +148,6 @@ static void update_barrings(struct ofono_call_barring *cb, int mask)
 
 	for (i = cb->query_start; i <= cb->query_end; i++) {
 		cb->cur_locks[i] = cb->new_locks[i];
-		cb->new_locks[i] = 0;
 	}
 }
 
@@ -164,7 +163,7 @@ static void cb_ss_property_append(struct ofono_call_barring *cb,
 		if (!(mask & i))
 			continue;
 
-		strvalue = (cb->cur_locks[lock] & i) ? "enabled" : "disabled";
+		strvalue = (cb->new_locks[lock] & i) ? "enabled" : "disabled";
 
 		snprintf(property_name, sizeof(property_name), "%s%s",
 				bearer_class_to_string(i),
@@ -590,7 +589,7 @@ static inline void cb_append_property(struct ofono_call_barring *cb,
 	int i;
 
 	for (i = start; i <= end; i++)
-		if (cb->cur_locks[i] & cls)
+		if (cb->new_locks[i] & cls)
 			break;
 
 	if (i <= end)
-- 
1.6.3.3

[PATCH 3/7] Number of barring services is constant.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 src/call-barring.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/call-barring.c b/src/call-barring.c
index 530d64b..eedb41d 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -37,6 +37,7 @@
 #include "common.h"
 
 #define CALL_BARRING_FLAG_CACHED 0x1
+#define NUM_OF_BARRINGS 5
 
 static GSList *g_drivers = NULL;
 
@@ -47,8 +48,8 @@ static void set_query_next_lock(struct ofono_call_barring *cb);
 struct ofono_call_barring {
 	int flags;
 	DBusMessage *pending;
-	int *cur_locks;
-	int *new_locks;
+	int cur_locks[NUM_OF_BARRINGS];
+	int new_locks[NUM_OF_BARRINGS];
 	int query_start;
 	int query_end;
 	int query_next;
@@ -1070,9 +1071,6 @@ static void call_barring_remove(struct ofono_atom *atom)
 	if (cb->driver && cb->driver->remove)
 		cb->driver->remove(cb);
 
-	g_free(cb->cur_locks);
-	g_free(cb->new_locks);
-
 	g_free(cb);
 }
 
@@ -1083,7 +1081,6 @@ struct ofono_call_barring *ofono_call_barring_create(struct ofono_modem *modem,
 {
 	struct ofono_call_barring *cb;
 	GSList *l;
-	int lcount;
 
 	if (driver == NULL)
 		return NULL;
@@ -1093,10 +1090,6 @@ struct ofono_call_barring *ofono_call_barring_create(struct ofono_modem *modem,
 	if (cb == NULL)
 		return NULL;
 
-	lcount = CB_ALL_END - CB_ALL_START + 1;
-
-	cb->cur_locks = g_new0(int, lcount);
-	cb->new_locks = g_new0(int, lcount);
 	cb->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_CALL_BARRING,
 						call_barring_remove, cb);
 
-- 
1.6.3.3

[PATCH 4/7] Network password has exactly 4 digits.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 src/call-barring.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/call-barring.c b/src/call-barring.c
index eedb41d..f3252e1 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -360,6 +360,12 @@ static const char *cb_ss_service_to_fac(const char *svc)
 	return NULL;
 }
 
+/* Network password has exactly 4 digits */
+static inline int is_valid_password(const char *pass)
+{
+	return is_valid_pin(pass) && strlen(pass) == 4;
+}
+
 static gboolean cb_ss_control(int type, const char *sc,
 				const char *sia, const char *sib,
 				const char *sic, const char *dn,
@@ -403,7 +409,7 @@ static gboolean cb_ss_control(int type, const char *sc,
 	if (strlen(dn) > 0)
 		goto bad_format;
 
-	if (!is_valid_pin(sia))
+	if (!is_valid_password(sia))
 		goto bad_format;
 
 	switch (type) {
@@ -524,7 +530,7 @@ static gboolean cb_ss_passwd(const char *sc,
 	if (!fac)
 		return FALSE;
 
-	if (!is_valid_pin(old) || !is_valid_pin(new))
+	if (!is_valid_password(old) || !is_valid_password(new))
 		goto bad_format;
 
 	cb->pending = dbus_message_ref(msg);
@@ -855,7 +861,7 @@ static DBusMessage *cb_set_property(DBusConnection *conn, DBusMessage *msg,
 			return __ofono_error_invalid_args(msg);
 
 		dbus_message_iter_get_basic(&iter, &passwd);
-		if (!is_valid_pin(passwd))
+		if (!is_valid_password(passwd))
 			return __ofono_error_invalid_format(msg);
 	}
 
@@ -902,7 +908,7 @@ static DBusMessage *cb_disable_all(DBusConnection *conn, DBusMessage *msg,
 					DBUS_TYPE_INVALID) == FALSE)
 		return __ofono_error_invalid_args(msg);
 
-	if (!is_valid_pin(passwd))
+	if (!is_valid_password(passwd))
 		return __ofono_error_invalid_format(msg);
 
 	cb_set_query_bounds(cb, fac, FALSE);
@@ -950,10 +956,10 @@ static DBusMessage *cb_set_passwd(DBusConnection *conn, DBusMessage *msg,
 					DBUS_TYPE_INVALID) == FALSE)
 		return __ofono_error_invalid_args(msg);
 
-	if (!is_valid_pin(old_passwd))
+	if (!is_valid_password(old_passwd))
 		return __ofono_error_invalid_format(msg);
 
-	if (!is_valid_pin(new_passwd))
+	if (!is_valid_password(new_passwd))
 		return __ofono_error_invalid_format(msg);
 
 	cb->pending = dbus_message_ref(msg);
-- 
1.6.3.3

[PATCH 5/7] Interrogation of a barring does not require password.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 src/call-barring.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/call-barring.c b/src/call-barring.c
index f3252e1..a0f961e 100644
--- a/src/call-barring.c
+++ b/src/call-barring.c
@@ -409,7 +409,7 @@ static gboolean cb_ss_control(int type, const char *sc,
 	if (strlen(dn) > 0)
 		goto bad_format;
 
-	if (!is_valid_password(sia))
+	if (type != SS_CONTROL_TYPE_QUERY && !is_valid_password(sia))
 		goto bad_format;
 
 	switch (type) {
-- 
1.6.3.3

[PATCH 6/7] Added call-barring-api.txt.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 doc/call-barring-api.txt |   65 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)
 create mode 100644 doc/call-barring-api.txt

diff --git a/doc/call-barring-api.txt b/doc/call-barring-api.txt
new file mode 100644
index 0000000..644415b
--- /dev/null
+++ b/doc/call-barring-api.txt
@@ -0,0 +1,65 @@
+Call Barring hierarchy
+======================
+
+Service		org.ofono
+Interface	org.ofono.CallBarring
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		dict GetProperties()
+
+			Contains the properties for this object.
+
+		void ChangePassword(string old_password, string new_password)
+
+			Register new network password for the barring
+			services.
+
+		void DisableAll(string password)
+
+			Disables all call barrings.
+
+		void DisableAllIncoming(string password)
+
+			Disables barrings for incoming calls.
+
+		void DisableAllOutgoing(string password)
+
+			Disables barrings for outgoing calls.
+
+		void SetProperty(string property, variant value)
+
+			Sets the given property value to that specified in
+			call parameter.
+
+Signals		PropertyChanged(string property, variant value)
+
+			Signal is emitted whenever a property has changed.  The new
+			value is passed as the signal argument.
+
+		IncomingBarringInEffect()
+		
+			Signal is emitted when a call is made and an
+			incoming call barring supplementary service is in use.
+
+		OutgoingBarringInEffect()
+		
+			Signal is emitted when a call is made and an
+			outgoing call barring supplementary service is in use.
+
+Properties	string VoiceIncoming [readwrite]
+
+			Contains the value of the barrings for the incoming
+			voice calls. The possible values are:
+			- "always" bar all incoming voice calls
+			- "whenroaming" bar incoming voice calls when roaming,
+			- "disabled" if no barring is active
+
+		string VoiceOutgoing [readwrite]
+
+			Contains the value of the barrings for the outgoing
+			voice calls. The possible values are:
+			- "all" bar all outgoing calls
+			- "international" bar all outgoing international calls
+			- "internationalnothome" bar all outgoing
+			  international calls except to home country
+			- "disabled" if no barring is active
-- 
1.6.3.3

[PATCH 7/7] Fix: isimodem handling of call barring services.

[ppessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>


Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
---
 drivers/isimodem/call-barring.c |   64 ++++++++++++++++++++------------------
 drivers/isimodem/ss.h           |   28 +++++++++--------
 2 files changed, 49 insertions(+), 43 deletions(-)

diff --git a/drivers/isimodem/call-barring.c b/drivers/isimodem/call-barring.c
index 6487ae8..6261b23 100644
--- a/drivers/isimodem/call-barring.c
+++ b/drivers/isimodem/call-barring.c
@@ -85,14 +85,13 @@ static void isi_set(struct ofono_call_barring *barr, const char *lock,
 	struct barr_data *bd = ofono_call_barring_get_data(barr);
 	struct isi_cb_data *cbd = isi_cb_data_new(barr, cb, data);
 	int ss_code;
-	char *ucs2 = NULL;
 
 	unsigned char msg[] = {
 		SS_SERVICE_REQ,
 		enable ? SS_ACTIVATION : SS_DEACTIVATION,
 		SS_ALL_TELE_AND_BEARER,
 		0, 0,				/* Supplementary services code */
-		SS_SEND_ADDITIONAL_INFO,
+		SS_SEND_ADDITIONAL_INFO,	/* ? */
 		1,				/* Subblock count */
 		SS_GSM_PASSWORD,
 		28,				/* Subblock length */
@@ -105,7 +104,9 @@ static void isi_set(struct ofono_call_barring *barr, const char *lock,
 	DBG("lock code %s enable %d class %d password %s\n",
 		lock, enable, cls, passwd);
 
-	if (!cbd || !passwd || strlen(passwd) > 4 || cls != 7)
+	if (!cbd || !lock)
+		goto error;
+	if (!passwd || strlen(passwd) != 4 || strspn(passwd, "0123456789") != 4)
 		goto error;
 
 	if (strcmp(lock, "AO") == 0)
@@ -120,23 +121,23 @@ static void isi_set(struct ofono_call_barring *barr, const char *lock,
 		ss_code = SS_GSM_BARR_ALL_IN_ROAM;
 	else if (strcmp(lock, "AB") == 0)
 		ss_code = SS_GSM_ALL_BARRINGS;
+	else if (enable)
+		/* Only Deactivate supports AG (333) and AC (353) */
+		goto error;
 	else if (strcmp(lock, "AG") == 0)
-		ss_code = SS_GSM_BARR_ALL_OUT;
+		ss_code = SS_GSM_OUTGOING_BARR_SERV;
 	else if (strcmp(lock, "AC") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN;
+		ss_code = SS_GSM_INCOMING_BARR_SERV;
 	else
 		goto error;
 
 	msg[3] = ss_code >> 8;
 	msg[4] = ss_code & 0xFF;
 
-	ucs2 = g_convert(passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
-
-	memcpy((char *)msg + 9, ucs2, 8);
-	g_free(ucs2);
+	msg[10] = passwd[0];
+	msg[12] = passwd[1];
+	msg[14] = passwd[2];
+	msg[16] = passwd[4];
 
 	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
 				set_resp_cb, cbd))
@@ -274,13 +275,13 @@ static void isi_query(struct ofono_call_barring *barr, const char *lock, int cls
 		SS_INTERROGATION,
 		SS_ALL_TELE_AND_BEARER,
 		0, 0,				/* Supplementary services code */
-		SS_SEND_ADDITIONAL_INFO,
+		SS_SEND_ADDITIONAL_INFO,	/* Get BER-encoded result, too */
 		0				/* Subblock count */
 	};
 
 	DBG("barring query lock code %s class %d\n", lock, cls);
 
-	if (!cbd || cls != 7)
+	if (!cbd)
 		goto error;
 
 	if (strcmp(lock, "AO") == 0)
@@ -344,7 +345,6 @@ static void isi_set_passwd(struct ofono_call_barring *barr, const char *lock,
 	struct barr_data *bd = ofono_call_barring_get_data(barr);
 	struct isi_cb_data *cbd = isi_cb_data_new(barr, cb, data);
 	int ss_code;
-	char *ucs2 = NULL;
 
 	unsigned char msg[] = {
 		SS_SERVICE_REQ,
@@ -361,7 +361,11 @@ static void isi_set_passwd(struct ofono_call_barring *barr, const char *lock,
 		0, 0				/* Filler */
 	};
 
-	if (!cbd || strlen(old_passwd) > 4 || strlen(new_passwd) > 4)
+	if (!cbd)
+		goto error;
+	if (strlen(old_passwd) != 4 || strspn(old_passwd, "0123456789") != 4)
+		goto error;
+	if (strlen(new_passwd) != 4 || strspn(new_passwd, "0123456789") != 4)
 		goto error;
 
 	DBG("lock code %s old password %s new password %s\n",
@@ -369,28 +373,28 @@ static void isi_set_passwd(struct ofono_call_barring *barr, const char *lock,
 
 	if (strcmp(lock, "AB") == 0)
 		ss_code = SS_GSM_ALL_BARRINGS;
+	else if (strcmp(lock, "") == 0)
+		ss_code = 0;
 	else
 		goto error;
 
 	msg[3] = ss_code >> 8;
 	msg[4] = ss_code & 0xFF;
 
-	ucs2 = g_convert(old_passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
-
-	memcpy((char *)msg + 9, ucs2, 8);
-	g_free(ucs2);
+	msg[10] = old_passwd[0];
+	msg[12] = old_passwd[1];
+	msg[14] = old_passwd[2];
+	msg[16] = old_passwd[4];
 
-	ucs2 = g_convert(new_passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
+	msg[18] = new_passwd[0];
+	msg[20] = new_passwd[1];
+	msg[22] = new_passwd[2];
+	msg[24] = new_passwd[4];
 
-	memcpy((char *)msg + 17, ucs2, 8);
-	memcpy((char *)msg + 25, ucs2, 8);
-	g_free(ucs2);
+	msg[26] = new_passwd[0];
+	msg[28] = new_passwd[1];
+	msg[30] = new_passwd[2];
+	msg[32] = new_passwd[4];
 
 	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
 				set_passwd_resp_cb, cbd))
diff --git a/drivers/isimodem/ss.h b/drivers/isimodem/ss.h
index 7c20bcc..2313ae6 100644
--- a/drivers/isimodem/ss.h
+++ b/drivers/isimodem/ss.h
@@ -81,19 +81,21 @@ enum ss_basic_service_codes {
 };
 
 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,
+	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,
 };
 
 enum ss_response_data {
-- 
1.6.3.3

Re: [PATCH 1/7] Barring services can only be activated/deactivated.

[Denis Kenzior]

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

Hi Pekka,

> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> 
> Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>

For future reference please remember that we do not use the Signed-off by in 
oFono.  Please configure your client not to include that.

> ---
>  src/call-barring.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 2/7] Return newly-queried barring status.

[Denis Kenzior]

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

Hi Pekka,

>  	for (i = cb->query_start; i <= cb->query_end; i++) {
>  		cb->cur_locks[i] = cb->new_locks[i];
> -		cb->new_locks[i] = 0;
>  	}

Can you also ensure that if a loop / if statement no longer requires braces 
that your patch also takes care of that too?  I fixed it up for you afterward.

Patch has been applied, thanks.

Regards,
-Denis

Regards,
-Denis

Re: [PATCH 3/7] Number of barring services is constant.

[Denis Kenzior]

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

Hi Pekka,

>  src/call-barring.c |   13 +++----------
>  1 files changed, 3 insertions(+), 10 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 4/7] Network password has exactly 4 digits.

[Denis Kenzior]

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

Hi Pekka,

> +/* Network password has exactly 4 digits */
> +static inline int is_valid_password(const char *pass)
> +{
> +	return is_valid_pin(pass) && strlen(pass) == 4;
> +}
> +

So I suggest not doing it this way, instead let us modify is_valid_pin to take 
an additional argument, namely whether this is a pin/puk and check the length 
of the password there.  You might want to include a reference to 22.004 
Section 5.2 that says the pin/pin2 is 0000 to 9999 and exactly 4 digits.

Regards,
-Denis

Re: [PATCH 5/7] Interrogation of a barring does not require password.

[Denis Kenzior]

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

Hi Pekka,

> +	if (type != SS_CONTROL_TYPE_QUERY && !is_valid_password(sia))
>  		goto bad_format;

I applied this patch with slight modification until is_valid_pin is fixed.

Regards,
-Denis

Re: [PATCH 6/7] Added call-barring-api.txt.

[Denis Kenzior]

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

Hi Pekka,

> ---
>  doc/call-barring-api.txt |   65
>  ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 65
>  insertions(+), 0 deletions(-)
>  create mode 100644 doc/call-barring-api.txt
> 

> +Signals		PropertyChanged(string property, variant value)
> +
> +			Signal is emitted whenever a property has changed.  The new
> +			value is passed as the signal argument.

This line was over 80 characters.  I fixed it up for you this time, but please 
keep this in mind.

Also, when applying the patch I got this:
Applying: Added call-barring-api.txt.
/home/denkenz/ofono-master/.git/rebase-apply/patch:51: trailing whitespace.
		
/home/denkenz/ofono-master/.git/rebase-apply/patch:56: trailing whitespace.

Please make sure that your editor is setup properly and does not introduce 
trailing whitespace.  Again I fixed it up for you this time around.

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 7/7] Fix: isimodem handling of call barring services.

[Denis Kenzior]

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

Hi Pekka,

> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> 
> Signed-off-by: Pekka Pessi <Pekka.Pessi@nokia.com>
> ---
>  drivers/isimodem/call-barring.c |   64
>  ++++++++++++++++++++------------------ drivers/isimodem/ss.h           |  
>  28 +++++++++--------
>  2 files changed, 49 insertions(+), 43 deletions(-)
> @@ -105,7 +104,9 @@ static void isi_set(struct ofono_call_barring *barr,
>  const char *lock, DBG("lock code %s enable %d class %d password %s\n",
>  		lock, enable, cls, passwd);
> 
> -	if (!cbd || !passwd || strlen(passwd) > 4 || cls != 7)
> +	if (!cbd || !lock)
> +		goto error;
> +	if (!passwd || strlen(passwd) != 4 || strspn(passwd, "0123456789") != 4)
>  		goto error;

I suggest not doing this checking in the driver code, if the core is sending 
you something you don't expect, let us fix the core.

> 
>  	if (strcmp(lock, "AO") == 0)
> @@ -120,23 +121,23 @@ static void isi_set(struct ofono_call_barring *barr,
>  const char *lock, ss_code = SS_GSM_BARR_ALL_IN_ROAM;
>  	else if (strcmp(lock, "AB") == 0)
>  		ss_code = SS_GSM_ALL_BARRINGS;
> +	else if (enable)
> +		/* Only Deactivate supports AG (333) and AC (353) */
> +		goto error;

Same here, to my knowledge the core never actually allows this.

> -	if (!cbd || strlen(old_passwd) > 4 || strlen(new_passwd) > 4)
> +	if (!cbd)
> +		goto error;
> +	if (strlen(old_passwd) != 4 || strspn(old_passwd, "0123456789") != 4)
> +		goto error;
> +	if (strlen(new_passwd) != 4 || strspn(new_passwd, "0123456789") != 4)
>  		goto error;

Again, let us fix the core so you guys don't have to do this checking.

Regards,
-Denis

Re: [PATCH 7/7] Fix: isimodem handling of call barring services.

[Pekka Pessi]

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

Hi Denis,

2010/3/18 Denis Kenzior <denkenz@gmail.com>:
>> -     if (!cbd || !passwd || strlen(passwd) > 4 || cls != 7)
>> +     if (!cbd || !lock)
>> +             goto error;
>> +     if (!passwd || strlen(passwd) != 4 || strspn(passwd, "0123456789") != 4)
>>               goto error;
>
> I suggest not doing this checking in the driver code, if the core is sending
> you something you don't expect, let us fix the core.

Do you suggest removing all the current checks, and leaving only the
checks for possible features the driver does not support?

--Pekka

-- 
Pekka.Pessi mail at nokia.com

Re: [PATCH 7/7] Fix: isimodem handling of call barring services.

[Denis Kenzior]

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

Hi Pekka,

> Hi Denis,
> 
> 2010/3/18 Denis Kenzior <denkenz@gmail.com>:
> >> -     if (!cbd || !passwd || strlen(passwd) > 4 || cls != 7)
> >> +     if (!cbd || !lock)
> >> +             goto error;
> >> +     if (!passwd || strlen(passwd) != 4 || strspn(passwd, "0123456789")
> >> != 4) goto error;
> >
> > I suggest not doing this checking in the driver code, if the core is
> > sending you something you don't expect, let us fix the core.
> 
> Do you suggest removing all the current checks, and leaving only the
> checks for possible features the driver does not support?

Correct.  The reason is that I want to avoid the situation of each driver 
doing its own checking.  This quickly leads to a bunch of useless code and 
most of it ends up incorrect or with only partial coverage.

If the core is sending things which are not valid, then the core has to be 
fixed.  This automatically fixes all the drivers too.

Regards,
-Denis

Re: [PATCH 4/7] Network password has exactly 4 digits.

[Pekka Pessi]

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

2010/3/18 Denis Kenzior <denkenz@gmail.com>:
>> +static inline int is_valid_password(const char *pass)
>> +{
>> +     return is_valid_pin(pass) && strlen(pass) == 4;
>> +}

> So I suggest not doing it this way, instead let us modify is_valid_pin to take
> an additional argument, namely whether this is a pin/puk and check the length
> of the password there.  You might want to include a reference to 22.004
> Section 5.2 that says the pin/pin2 is 0000 to 9999 and exactly 4 digits.

OK, sent a patch fixing is_valid_pin().

-- 
Pekka.Pessi mail at nokia.com