[PATCH 1/2] netreg: Add CPHS CSP implementation

[Aki Niemi]

---
 src/network.c |  121 
 1 files changed, 95 insertions(+), 26 deletions(-)

diff --git a/src/network.c b/src/network.c
index bf61472..ef0b37f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -42,7 +42,8 @@
 
 enum network_registration_mode {
NETWORK_REGISTRATION_MODE_AUTO =0,
-   NETWORK_REGISTRATION_MODE_MANUAL =  1,
+   NETWORK_REGISTRATION_MODE_AUTO_ONLY =   1,
+   NETWORK_REGISTRATION_MODE_MANUAL =  2,
 };
 
 #define SETTINGS_STORE netreg
@@ -98,6 +99,8 @@ static const char *registration_mode_to_string(int mode)
switch (mode) {
case NETWORK_REGISTRATION_MODE_AUTO:
return auto;
+   case NETWORK_REGISTRATION_MODE_AUTO_ONLY:
+   return auto-only;
case NETWORK_REGISTRATION_MODE_MANUAL:
return manual;
}
@@ -143,6 +146,42 @@ static char **network_operator_technologies(struct 
network_operator_data *opd)
return techs;
 }
 
+static void registration_status_callback(const struct ofono_error *error,
+   int status, int lac, int ci, int tech,
+   void *data)
+{
+   struct ofono_netreg *netreg = data;
+
+   if (error-type != OFONO_ERROR_TYPE_NO_ERROR) {
+   DBG(Error during registration status query);
+   return;
+   }
+
+   ofono_netreg_status_notify(netreg, status, lac, ci, tech);
+}
+
+static void init_register(const struct ofono_error *error, void *data)
+{
+   struct ofono_netreg *netreg = data;
+
+   if (netreg-driver-registration_status == NULL)
+   return;
+
+   netreg-driver-registration_status(netreg,
+   registration_status_callback, netreg);
+}
+
+static void enforce_auto_only(struct ofono_netreg *netreg)
+{
+   if (netreg-mode != NETWORK_REGISTRATION_MODE_MANUAL)
+   return;
+
+   if (netreg-driver-register_auto == NULL)
+   return;
+
+   netreg-driver-register_auto(netreg, init_register, netreg);
+}
+
 static void set_registration_mode(struct ofono_netreg *netreg, int mode)
 {
DBusConnection *conn;
@@ -152,6 +191,9 @@ static void set_registration_mode(struct ofono_netreg 
*netreg, int mode)
if (netreg-mode == mode)
return;
 
+   if (mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+   enforce_auto_only(netreg);
+
netreg-mode = mode;
 
if (netreg-settings) {
@@ -170,20 +212,6 @@ static void set_registration_mode(struct ofono_netreg 
*netreg, int mode)
Mode, DBUS_TYPE_STRING, strmode);
 }
 
-static void registration_status_callback(const struct ofono_error *error,
-   int status, int lac, int ci, int tech,
-   void *data)
-{
-   struct ofono_netreg *netreg = data;
-
-   if (error-type != OFONO_ERROR_TYPE_NO_ERROR) {
-   DBG(Error during registration status query);
-   return;
-   }
-
-   ofono_netreg_status_notify(netreg, status, lac, ci, tech);
-}
-
 static void register_callback(const struct ofono_error *error, void *data)
 {
struct ofono_netreg *netreg = data;
@@ -211,15 +239,6 @@ out:
registration_status_callback, netreg);
 }
 
-static void init_register(const struct ofono_error *error, void *data)
-{
-   struct ofono_netreg *netreg = data;
-
-   if (netreg-driver-registration_status)
-   netreg-driver-registration_status(netreg,
-   registration_status_callback, netreg);
-}
-
 static struct network_operator_data *
network_operator_create(const struct ofono_network_operator *op)
 {
@@ -586,6 +605,9 @@ static DBusMessage 
*network_operator_register(DBusConnection *conn,
struct network_operator_data *opd = data;
struct ofono_netreg *netreg = opd-netreg;
 
+   if (netreg-mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+   return __ofono_error_access_denied(msg);
+
if (netreg-pending)
return __ofono_error_busy(msg);
 
@@ -838,6 +860,9 @@ static DBusMessage *network_register(DBusConnection *conn,
 
netreg-driver-register_auto(netreg, register_callback, netreg);
 
+   if (netreg-mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+   return NULL;
+
set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);
 
return NULL;
@@ -947,6 +972,9 @@ static DBusMessage *network_scan(DBusConnection *conn,
 {
struct ofono_netreg *netreg = data;
 
+   if (netreg-mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+   return __ofono_error_access_denied(msg);
+
if (netreg-pending)
return __ofono_error_busy(msg);
 
@@ -1364,7 +1392,7 @@ static void init_registration_status(const struct 

Re: [PATCH 1/2] netreg: Add CPHS CSP implementation

[Denis Kenzior]

Hi Aki,

 @@ -838,6 +860,9 @@ static DBusMessage *network_register(DBusConnection *conn,
  
   netreg-driver-register_auto(netreg, register_callback, netreg);
  
 + if (netreg-mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
 + return NULL;
 +

So we might just return access_denied here as well.

   set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);
  
   return NULL;

snip

 @@ -1769,7 +1835,7 @@ static void netreg_load_settings(struct ofono_netreg 
 *netreg)
   mode = g_key_file_get_integer(netreg-settings, SETTINGS_GROUP,
   Mode, NULL);
  
 - if (mode = 0  mode = 1)
 + if (mode != NETWORK_REGISTRATION_MODE_AUTO_ONLY)
   netreg-mode = mode;
  

This check is unnecessary, you already loaded the settings before you
managed to load the sim file.  You're also removing a sanity check for
no real reason.

This brings up another point, since the CPHS CSP file is actually
user-writeable (for some unknown reason) the user can remove the SIM,
put it in another phone and modify the forced-auto settings.  If you
store the forced-auto mode in the settings file, then there's no way to
ever go back to non-forced auto, even though he should be able to.

I suggest that when forced-auto is detected, that we only write
NETWORK_REGISTRATION_MODE_AUTO to the settings store.

Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH 1/2] netreg: Add CPHS CSP implementation

[Aki Niemi]

Hi Denis,

2011/2/3 Denis Kenzior denk...@gmail.com:
 +     if (netreg-mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
 +             return NULL;
 +

 So we might just return access_denied here as well.

There is a reason for allowing this, namely that sometimes you want to
re-run the logic for automatically operator selection. Granted, this
is not your most common use case, but since it also does no harm to
allowing it, I didn't.

       set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);

       return NULL;

 snip

 @@ -1769,7 +1835,7 @@ static void netreg_load_settings(struct ofono_netreg 
 *netreg)
       mode = g_key_file_get_integer(netreg-settings, SETTINGS_GROUP,
                                       Mode, NULL);

 -     if (mode = 0  mode = 1)
 +     if (mode != NETWORK_REGISTRATION_MODE_AUTO_ONLY)
               netreg-mode = mode;


 This check is unnecessary, you already loaded the settings before you
 managed to load the sim file.  You're also removing a sanity check for
 no real reason.

That's just a bug. The intention was to write:

if (mode == NEtWORK_REGISTRATION_MODE_AUTO ||
mode == NETWORK_REGISTRATION_MODE_MANUAL)
netreg-mode = mode;

The patch has a seemingly shorter way of describing the same, but as
you point out, misses the sanity check. Must have been lack of coffee
or something. ;)

 This brings up another point, since the CPHS CSP file is actually
 user-writeable (for some unknown reason) the user can remove the SIM,
 put it in another phone and modify the forced-auto settings.  If you
 store the forced-auto mode in the settings file, then there's no way to
 ever go back to non-forced auto, even though he should be able to.

I think the above way will in fact default to auto even if the
settings store has auto-only. And yes, EFcsp can definitely change
even at run-time, as manual mode really should be allowed when
roaming, at least in countries that the operator for instance has
multiple suitable roaming partners with varying coverage.

 I suggest that when forced-auto is detected, that we only write
 NETWORK_REGISTRATION_MODE_AUTO to the settings store.

That sounds reasonable, too.

Cheers,
Aki
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono