From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCH] sim: enable usage of SIM pass codes longer than 8 digits
Date: Mon, 07 Feb 2011 22:17:32 -0600 [thread overview]
Message-ID: <4D50C3DC.9020209@gmail.com> (raw)
In-Reply-To: <1297090177-26010-1-git-send-email-jussi.kangas@tieto.com>
[-- Attachment #1: Type: text/plain, Size: 10774 bytes --]
Hi Jussi,
On 02/07/2011 08:49 AM, Jussi Kangas wrote:
> ---
>
> Hi,
>
> Here is my second proposal for how to enable usage of SIM lock codes longer than eight digits. I removed the namespace problem and fixed a problem with puk reset.
>
> Br,
> Jussi
>
> include/sim.h | 1 +
> src/call-barring.c | 14 ++++++++------
> src/call-meter.c | 4 ++--
> src/common.c | 29 ++++++++++++++++++++++++-----
> src/common.h | 11 +++--------
> src/ofono.h | 3 +++
> src/sim.c | 15 +++++++++------
> 7 files changed, 50 insertions(+), 27 deletions(-)
>
> diff --git a/include/sim.h b/include/sim.h
> index 5e3ba5b..7f0313e 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -54,6 +54,7 @@ enum ofono_sim_password_type {
> OFONO_SIM_PASSWORD_PHNETSUB_PUK,
> OFONO_SIM_PASSWORD_PHSP_PUK,
> OFONO_SIM_PASSWORD_PHCORP_PUK,
> + OFONO_SIM_PASSWORD_PIN_TYPE_NET,
So just adding an enum here is a little dangerous since we use the array
size for things like look up tables and iterators inside src/sim.c
> OFONO_SIM_PASSWORD_INVALID,
> };
>
> diff --git a/src/call-barring.c b/src/call-barring.c
> index 649826e..fdcecbf 100644
> --- a/src/call-barring.c
> +++ b/src/call-barring.c
> @@ -402,7 +402,8 @@ static gboolean cb_ss_control(int type, const char *sc,
> if (strlen(dn) > 0)
> goto bad_format;
>
> - if (type != SS_CONTROL_TYPE_QUERY && !is_valid_pin(sia, PIN_TYPE_NET))
> + if (type != SS_CONTROL_TYPE_QUERY &&
> + !is_valid_pin(sia, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
Watch out for coding style, see item M4.
> goto bad_format;
>
> switch (type) {
> @@ -524,7 +525,8 @@ static gboolean cb_ss_passwd(const char *sc,
> if (fac == NULL)
> return FALSE;
>
> - if (!is_valid_pin(old, PIN_TYPE_NET) || !is_valid_pin(new, PIN_TYPE_NET))
> + if (!is_valid_pin(old, OFONO_SIM_PASSWORD_PIN_TYPE_NET) ||
> + !is_valid_pin(new, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
As above
> goto bad_format;
>
> cb->pending = dbus_message_ref(msg);
> @@ -862,7 +864,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, PIN_TYPE_NET))
> + if (!is_valid_pin(passwd, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
> return __ofono_error_invalid_format(msg);
> }
>
> @@ -909,7 +911,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, PIN_TYPE_NET))
> + if (!is_valid_pin(passwd, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
> return __ofono_error_invalid_format(msg);
>
> cb_set_query_bounds(cb, fac, FALSE);
> @@ -957,10 +959,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, PIN_TYPE_NET))
> + if (!is_valid_pin(old_passwd, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(new_passwd, PIN_TYPE_NET))
> + if (!is_valid_pin(new_passwd, OFONO_SIM_PASSWORD_PIN_TYPE_NET))
> return __ofono_error_invalid_format(msg);
>
> cb->pending = dbus_message_ref(msg);
> diff --git a/src/call-meter.c b/src/call-meter.c
> index d483e2e..a7f8ebb 100644
> --- a/src/call-meter.c
> +++ b/src/call-meter.c
> @@ -549,7 +549,7 @@ static DBusMessage *cm_set_property(DBusConnection *conn, DBusMessage *msg,
>
> dbus_message_iter_get_basic(&iter, &passwd);
>
> - if (!is_valid_pin(passwd, PIN_TYPE_PIN))
> + if (!is_valid_pin(passwd, OFONO_SIM_PASSWORD_SIM_PIN2))
> return __ofono_error_invalid_format(msg);
>
> for (property = cm_properties; property->name; property++) {
> @@ -621,7 +621,7 @@ static DBusMessage *cm_acm_reset(DBusConnection *conn, DBusMessage *msg,
> DBUS_TYPE_INVALID) == FALSE)
> return __ofono_error_invalid_args(msg);
>
> - if (!is_valid_pin(pin2, PIN_TYPE_PIN))
> + if (!is_valid_pin(pin2, OFONO_SIM_PASSWORD_SIM_PIN2))
> return __ofono_error_invalid_format(msg);
>
> cm->pending = dbus_message_ref(msg);
> diff --git a/src/common.c b/src/common.c
> index f25f105..60bf20c 100644
> --- a/src/common.c
> +++ b/src/common.c
> @@ -649,7 +649,7 @@ const char *bearer_class_to_string(enum bearer_class cls)
> return NULL;
> }
>
> -gboolean is_valid_pin(const char *pin, enum pin_type type)
> +gboolean is_valid_pin(const char *pin, enum ofono_sim_password_type type)
Why don't we keep things simple. Modify is_valid_pin to take a pin and
a min and max number of digits.
gboolean is_valid_pin_with_limits(const char *pin, int min, int max)
(feel free to pick some better name)
Then just add two functions:
__ofono_valid_net_pin(const char *pin)
__ofono_valid_sim_pin(const char *pin, enum ofono_sim_password_type type)
Stick both in ofono.h / sim.c somewhere
> {
> unsigned int i;
>
> @@ -662,25 +662,44 @@ gboolean is_valid_pin(const char *pin, enum pin_type type)
> return FALSE;
>
> switch (type) {
> - case PIN_TYPE_PIN:
> + case OFONO_SIM_PASSWORD_SIM_PIN:
> + case OFONO_SIM_PASSWORD_SIM_PIN2:
> /* 11.11 Section 9.3 ("CHV"): 4..8 IA-5 digits */
> if (4 <= i && i <= 8)
> return TRUE;
> break;
> - case PIN_TYPE_PUK:
> + case OFONO_SIM_PASSWORD_PHSIM_PIN:
> + case OFONO_SIM_PASSWORD_PHFSIM_PIN:
> + case OFONO_SIM_PASSWORD_PHNET_PIN:
> + case OFONO_SIM_PASSWORD_PHNETSUB_PIN:
> + case OFONO_SIM_PASSWORD_PHSP_PIN:
> + case OFONO_SIM_PASSWORD_PHCORP_PIN:
> + /* 22.022 Section 14 4..16 IA-5 digits */
> + if (4 <= i && i <= 16)
> + return TRUE;
> + break;
> + case OFONO_SIM_PASSWORD_SIM_PUK:
> + case OFONO_SIM_PASSWORD_SIM_PUK2:
> + case OFONO_SIM_PASSWORD_PHFSIM_PUK:
> + case OFONO_SIM_PASSWORD_PHNET_PUK:
> + case OFONO_SIM_PASSWORD_PHNETSUB_PUK:
> + case OFONO_SIM_PASSWORD_PHSP_PUK:
> + case OFONO_SIM_PASSWORD_PHCORP_PUK:
> /* 11.11 Section 9.3 ("UNBLOCK CHV"), 8 IA-5 digits */
> if (i == 8)
> return TRUE;
> break;
> - case PIN_TYPE_NET:
> + case OFONO_SIM_PASSWORD_PIN_TYPE_NET:
> /* 22.004 Section 5.2, 4 IA-5 digits */
> if (i == 4)
> return TRUE;
> break;
> - case PIN_TYPE_NONE:
> + case OFONO_SIM_PASSWORD_NONE:
> if (i < 8)
> return TRUE;
> break;
> + case OFONO_SIM_PASSWORD_INVALID:
> + break;
> }
>
> return FALSE;
> diff --git a/src/common.h b/src/common.h
> index 09f2deb..acdbce5 100644
> --- a/src/common.h
> +++ b/src/common.h
> @@ -19,6 +19,8 @@
> *
> */
>
> +#include "ofono.h"
> +
Please don't do that, common.h is supposed to be semi-independent from
the rest of the core so it can be unit-tested separately. I'm only
allowing inclusion of types.h here. Maybe this is a bad idea, and I
will change my mind later, but for now I'd like to stick to this.
> /* 27.007 Section 7.3 <AcT> */
> enum access_technology {
> ACCESS_TECHNOLOGY_GSM = 0,
> @@ -122,13 +124,6 @@ enum ss_cssu {
> SS_MT_CALL_DEFLECTED = 9,
> };
>
> -enum pin_type {
> - PIN_TYPE_NONE,
> - PIN_TYPE_PIN,
> - PIN_TYPE_PUK,
> - PIN_TYPE_NET,
> -};
> -
> /* 27.007 Section 10.1.10 */
> enum context_status {
> CONTEXT_STATUS_DEACTIVATED = 0,
> @@ -162,7 +157,7 @@ const char *ss_control_type_to_string(enum ss_control_type type);
>
> const char *bearer_class_to_string(enum bearer_class cls);
>
> -gboolean is_valid_pin(const char *pin, enum pin_type type);
> +gboolean is_valid_pin(const char *pin, enum ofono_sim_password_type type);
>
> const char *registration_status_to_string(int status);
> const char *registration_tech_to_string(int tech);
> diff --git a/src/ofono.h b/src/ofono.h
> index 6ba0187..ddd1bb9 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -18,6 +18,8 @@
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> *
> */
> +#ifndef OFONO_H
> +#define OFONO_H
>
Please don't do this. We actually leave out the include guards for a
reason. Besides, this has no bearing on this patch.
> #include <glib.h>
>
> @@ -430,3 +432,4 @@ ofono_bool_t __ofono_gprs_provision_get_settings(const char *mcc,
> void __ofono_gprs_provision_free_settings(
> struct ofono_gprs_provision_data *settings,
> int count);
> +#endif
> diff --git a/src/sim.c b/src/sim.c
> index 41d7e1d..42d0f39 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -174,6 +174,7 @@ static gboolean password_is_pin(enum ofono_sim_password_type type)
> case OFONO_SIM_PASSWORD_PHCORP_PUK:
> case OFONO_SIM_PASSWORD_INVALID:
> case OFONO_SIM_PASSWORD_NONE:
> + case OFONO_SIM_PASSWORD_PIN_TYPE_NET:
> return FALSE;
> }
>
> @@ -675,7 +676,7 @@ static DBusMessage *sim_lock_or_unlock(struct ofono_sim *sim, int lock,
> type == OFONO_SIM_PASSWORD_SIM_PIN2)
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(pin, PIN_TYPE_PIN))
> + if (!is_valid_pin(pin, type))
> return __ofono_error_invalid_format(msg);
>
> sim->pending = dbus_message_ref(msg);
> @@ -747,10 +748,10 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
> if (password_is_pin(type) == FALSE)
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(old, PIN_TYPE_PIN))
> + if (!is_valid_pin(old, type))
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(new, PIN_TYPE_PIN))
> + if (!is_valid_pin(new, type))
> return __ofono_error_invalid_format(msg);
>
> if (!strcmp(new, old))
> @@ -802,7 +803,7 @@ static DBusMessage *sim_enter_pin(DBusConnection *conn, DBusMessage *msg,
> if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(pin, PIN_TYPE_PIN))
> + if (!is_valid_pin(pin, type))
> return __ofono_error_invalid_format(msg);
>
> sim->pending = dbus_message_ref(msg);
> @@ -1012,10 +1013,12 @@ static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
> if (type == OFONO_SIM_PASSWORD_NONE || type != sim->pin_type)
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(puk, PIN_TYPE_PUK))
> + if (!is_valid_pin(puk, type))
> return __ofono_error_invalid_format(msg);
>
> - if (!is_valid_pin(pin, PIN_TYPE_PIN))
> + type = puk2pin(type);
> +
> + if (!is_valid_pin(pin, type))
> return __ofono_error_invalid_format(msg);
>
> sim->pending = dbus_message_ref(msg);
Regards,
-Denis
next prev parent reply other threads:[~2011-02-08 4:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-07 14:49 [PATCH] sim: enable usage of SIM pass codes longer than 8 digits Jussi Kangas
2011-02-08 4:17 ` Denis Kenzior [this message]
-- strict thread matches above, loose matches on Subject: below --
2011-02-08 12:48 Jussi Kangas
2011-02-08 16:22 ` Denis Kenzior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D50C3DC.9020209@gmail.com \
--to=denkenz@gmail.com \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.