Hi Jeevaka
On Mon, Jan 17, 2011 at 4:29 PM, Jeevaka Badrappan
<jeevaka.badrap...@elektrobit.com> wrote:
> Adds ifx support for the remaining pin retries query
> ---
> drivers/atmodem/sim.c | 83 +++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 67 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
> index c5f4a44..3f81249 100644
> --- a/drivers/atmodem/sim.c
> +++ b/drivers/atmodem/sim.c
> @@ -56,6 +56,7 @@ static const char *crsm_prefix[] = { "+CRSM:", NULL };
> static const char *cpin_prefix[] = { "+CPIN:", NULL };
> static const char *clck_prefix[] = { "+CLCK:", NULL };
> static const char *huawei_cpin_prefix[] = { "^CPIN:", NULL };
> +static const char *xpincnt_prefix[] = { "+XPINCNT:", NULL };
> static const char *none_prefix[] = { NULL };
>
> static void at_crsm_info_cb(gboolean ok, GAtResult *result, gpointer
> user_data)
> @@ -522,40 +523,90 @@ error:
> CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
> }
>
> +static void xpincnt_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> + struct cb_data *cbd = user_data;
> + ofono_sim_pin_retries_cb_t cb = cbd->cb;
> + const char *final = g_at_result_final_response(result);
> + GAtResultIter iter;
> + struct ofono_error error;
> + int retries[OFONO_SIM_PASSWORD_INVALID];
> + size_t i;
> + static enum ofono_sim_password_type password_types[] = {
> + OFONO_SIM_PASSWORD_SIM_PIN,
> + OFONO_SIM_PASSWORD_SIM_PIN2,
> + OFONO_SIM_PASSWORD_SIM_PUK,
> + OFONO_SIM_PASSWORD_SIM_PUK2,
> + };
> +
> + decode_at_error(&error, final);
> +
> + if (!ok) {
> + cb(&error, NULL, cbd->data);
> + return;
> + }
> +
> + g_at_result_iter_init(&iter, result);
> +
> + if (!g_at_result_iter_next(&iter, "+XPINCNT:"))
> + goto error;
> +
> + memset(retries, -1, sizeof(retries));
this happens to work for -1, but bear in mind memset "fills the
first n bytes of the memory area". Considering sizeof(int) == 4, it
will set each byte to -1, which happens to be -1 considering all 4
bytes too. But imo this is just being lucky.
In my first version I had the same as you and Denis asked me to use a for loop.
<snip>
> static void at_pin_retries_query(struct ofono_sim *sim,
> ofono_sim_pin_retries_cb_t cb, void *data)
> {
> struct sim_data *sd = ofono_sim_get_data(sim);
> struct cb_data *cbd;
> - int retries[OFONO_SIM_PASSWORD_INVALID];
> - int i;
>
> DBG("");
>
> - switch (sd->vendor) {
> - case OFONO_VENDOR_HUAWEI:
> - cbd = cb_data_new(cb, data);
> + cbd = cb_data_new(cb, data);
> + if (cbd == NULL) {
> + CALLBACK_WITH_FAILURE(cb, NULL, data);
> + return;
> + }
>
> - if (cbd == NULL) {
> - CALLBACK_WITH_FAILURE(cb, NULL, data);
> + switch (sd->vendor) {
> + case OFONO_VENDOR_IFX:
> + if (g_at_chat_send(sd->chat, "AT+XPINCNT", xpincnt_prefix,
> + xpincnt_cb, cbd, g_free) > 0)
> return;
> - }
>
> + break;
> + case OFONO_VENDOR_HUAWEI:
> if (g_at_chat_send(sd->chat, "AT^CPIN?", huawei_cpin_prefix,
> huawei_cpin_cb, cbd, g_free) > 0)
> return;
>
> - g_free(cbd);
> -
> - CALLBACK_WITH_FAILURE(cb, NULL, data);
> break;
> -
> default:
> - for(i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++)
> - retries[i] = -1;
> -
> - CALLBACK_WITH_SUCCESS(cb, retries, data);
> + break;
> }
> +
> + g_free(cbd);
> +
> + CALLBACK_WITH_FAILURE(cb, NULL, data);
If vendor is not huawei nor ifx, why are we bothering to allocate cbd
and free it afterwards without using?
This also changes the semantics to return a failure if it's not
implemented. I'm not against since this version seems to be clearer.
However you might want to change src/sim.c:sim_pin_retries_query_cb()
so it doesn't call ofono_error() in this case.
regards,
Lucas De Marchi
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
