Hi Lucas,
On 01/05/2011 11:20 AM, Lucas De Marchi wrote:
> Check the remaining pin retries after each operation that might have
> changed it, i.e. locking, unlocking, reseting or changing pin.
> ---
> src/sim.c | 97
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 97 insertions(+), 0 deletions(-)
>
> diff --git a/src/sim.c b/src/sim.c
> index 335f611..5374da1 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -62,6 +62,8 @@ struct ofono_sim {
> enum ofono_sim_password_type pin_type;
> gboolean locked_pins[OFONO_SIM_PASSWORD_SIM_PUK]; /* Number of PINs */
>
> + int pin_retries[OFONO_SIM_PASSWORD_INVALID];
> +
> enum ofono_sim_phase phase;
> unsigned char mnc_length;
> enum ofono_sim_cphs_phase cphs_phase;
> @@ -248,6 +250,33 @@ static char **get_locked_pins(struct ofono_sim *sim)
> return ret;
> }
>
> +static void **get_pin_retries(struct ofono_sim *sim)
> +{
> + int i, nelem;
> + void **ret;
> +
> + for (i = 1, nelem = 0; i < OFONO_SIM_PASSWORD_INVALID; i++) {
> + if (sim->pin_retries[i] == -1)
> + continue;
> +
> + nelem+=1;
> + }
> +
> + ret = g_new0(void *, nelem * 2 + 1);
> +
> + nelem = 0;
> +
> + for (i = 1; i < OFONO_SIM_PASSWORD_INVALID; i++) {
> + if (sim->pin_retries[i] == -1)
> + continue;
> +
> + ret[nelem++] = (void *)(sim_passwd_name(i));
Do you really need parentheses around the sim_passwd_name call? Also,
you need a space after the cast. e.g. (void *) sim_passwd_name(i);
> + ret[nelem++] = &sim->pin_retries[i];
> + }
> +
> + return ret;
> +}
> +
> static char **get_service_numbers(GSList *service_numbers)
> {
> int nelem;
> @@ -287,6 +316,7 @@ static DBusMessage *sim_get_properties(DBusConnection
> *conn,
> char **service_numbers;
> char **locked_pins;
> const char *pin_name;
> + void **pin_retries;
> dbus_bool_t present = sim->state != OFONO_SIM_STATE_NOT_PRESENT;
> dbus_bool_t fdn;
> dbus_bool_t bdn;
> @@ -369,12 +399,63 @@ static DBusMessage *sim_get_properties(DBusConnection
> *conn,
> DBUS_TYPE_STRING,
> (void *) &pin_name);
>
> + pin_retries = get_pin_retries(sim);
> +
We're not always consistent, but I prefer to logically group operations
together. So there should be no empty lines between pin_retries,
ofono_dbus_dict_append_dict and g_free.
> + ofono_dbus_dict_append_dict(&dict, "Retries", DBUS_TYPE_BYTE,
> + &pin_retries);
> +
> + g_free(pin_retries);
> +
> done:
> dbus_message_iter_close_container(&iter, &dict);
>
> return reply;
> }
>
> +static void sim_pin_retries_query_cb(const struct ofono_error *error,
> + int retries[OFONO_SIM_PASSWORD_INVALID],
> + void *data)
> +{
> + struct ofono_sim *sim = data;
> + DBusConnection *conn = ofono_dbus_get_connection();
> + const char *path = __ofono_atom_get_path(sim->atom);
> + int i;
> + void **pin_retries;
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + ofono_error("Querying remaining pin retries failed");
> +
> + return;
> + }
> +
> + for (i = 1; i < OFONO_SIM_PASSWORD_INVALID; i++) {
> + if (retries[i] != sim->pin_retries[i])
> + break;
> + }
> +
> + if (i == OFONO_SIM_PASSWORD_INVALID)
> + return;
> +
I think the above for and if statements can be easily handled by using
memcmp.
> + memcpy(sim->pin_retries, retries, sizeof(sim->pin_retries));
> +
> + pin_retries = get_pin_retries(sim);
> +
> + ofono_dbus_signal_dict_property_changed(conn, path,
> + OFONO_SIM_MANAGER_INTERFACE, "Retries",
> + DBUS_TYPE_BYTE, &pin_retries);
> +
> + g_free(pin_retries);
Same comment as above about grouping.
> +}
> +
> +static void sim_pin_retries_check(struct ofono_sim *sim)
> +{
> + if (sim->driver->query_pin_retries == NULL)
> + return;
> +
> + sim->driver->query_pin_retries(sim, sim_pin_retries_query_cb, sim);
> +}
> +
> +
> static void msisdn_set_done(struct msisdn_set_request *req)
> {
> DBusMessage *reply;
> @@ -549,6 +630,8 @@ static void sim_locked_cb(struct ofono_sim *sim, gboolean
> locked)
> "LockedPins", DBUS_TYPE_STRING,
> &locked_pins);
> g_strfreev(locked_pins);
> +
> + sim_pin_retries_check(sim);
> }
>
> static void sim_unlock_cb(const struct ofono_error *error, void *data)
> @@ -557,7 +640,10 @@ static void sim_unlock_cb(const struct ofono_error
> *error, void *data)
>
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> DBusMessage *reply = __ofono_error_failed(sim->pending);
> +
> __ofono_dbus_pending_reply(&sim->pending, reply);
> + sim_pin_retries_check(sim);
> +
> return;
> }
>
> @@ -570,7 +656,10 @@ static void sim_lock_cb(const struct ofono_error *error,
> void *data)
>
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> DBusMessage *reply = __ofono_error_failed(sim->pending);
> +
> __ofono_dbus_pending_reply(&sim->pending, reply);
> + sim_pin_retries_check(sim);
> +
> return;
> }
>
> @@ -639,11 +728,16 @@ static void sim_change_pin_cb(const struct ofono_error
> *error, void *data)
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> __ofono_dbus_pending_reply(&sim->pending,
> __ofono_error_failed(sim->pending));
> +
> + sim_pin_retries_check(sim);
> +
> return;
> }
>
> __ofono_dbus_pending_reply(&sim->pending,
> dbus_message_new_method_return(sim->pending));
> +
> + sim_pin_retries_check(sim);
> }
>
> static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
> @@ -1594,6 +1688,8 @@ static void sim_pin_query_cb(const struct ofono_error
> *error,
> &pin_name);
> }
>
> + sim_pin_retries_check(sim);
> +
> checkdone:
> if (pin_type == OFONO_SIM_PASSWORD_NONE)
> sim_initialize_after_pin(sim);
> @@ -2196,6 +2292,7 @@ struct ofono_sim *ofono_sim_create(struct ofono_modem
> *modem,
> sim->phase = OFONO_SIM_PHASE_UNKNOWN;
> sim->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_SIM,
> sim_remove, sim);
> + memset(sim->pin_retries, -1, sizeof(sim->pin_retries));
>
> for (l = g_drivers; l; l = l->next) {
> const struct ofono_sim_driver *drv = l->data;
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
