Re: [PATCH] sim: change pin_retries array to unsigned char

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Cedric,

On 11/28/2014 08:34 AM, Cedric Jehasse wrote:
> From: Cedric Jehasse <cedric.jehasse@softathome.com>
>
> Had a problem on mips, the Retries dict in org.ofono.SimManager was 0
> for all pin-types.
> The problem is int* are passed to dbus-functions, while the type is
> DBUS_TYPE_BYTE. Changed the type of pin_retries throughout the code to
> match the byte type.
> ---
>   drivers/atmodem/sim.c         | 24 ++++++++++++------------
>   drivers/isimodem/uicc.c       |  2 +-
>   drivers/qmimodem/sim-legacy.c |  2 +-
>   drivers/qmimodem/sim.c        |  2 +-
>   include/sim.h                 |  2 +-
>   src/sim.c                     | 13 +++++++------
>   6 files changed, 23 insertions(+), 22 deletions(-)
>

I really rather not modify every single driver for this issue.  Besides, 
the actual problem is actually in the core.  We were being quite evil 
with how we created our dictionary entries.

Please try the following patch (attached) and tell me if it fixes this 
bug on your platform.

Regards,
-Denis

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-sim-Fix-pin-retries-bogus-values-on-some-arch-es.patch --]
[-- Type: text/x-patch, Size: 3871 bytes --]

>From 7359b0322d93f6a332000c1064258d299c533bc3 Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Wed, 3 Dec 2014 17:31:09 -0600
Subject: [PATCH] sim: Fix pin retries bogus values on some arch-es

On some architectures the SimManager.Retries property was getting bogus
values.  This is because we were sending an array which pointed to int
values instead of the expected unsigned char values.

This fix allocates a temporary array of unsigned chars to hold the
actual D-Bus values being sent.  Additionally, the dictionary array is
changed to point to the temporary unsigned char based values instead of
the raw 'int' based retry values.
---
 src/sim.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index edae5eb..6e9baa9 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -267,10 +267,12 @@ static char **get_locked_pins(struct ofono_sim *sim)
 	return ret;
 }
 
-static void **get_pin_retries(struct ofono_sim *sim)
+static void get_pin_retries(struct ofono_sim *sim, void ***out_dict,
+				unsigned char **out_retries)
 {
 	int i, nelem;
-	void **ret;
+	void **dict;
+	unsigned char *retries;
 
 	for (i = 1, nelem = 0; i < OFONO_SIM_PASSWORD_INVALID; i++) {
 		if (sim->pin_retries[i] == -1)
@@ -279,17 +281,21 @@ static void **get_pin_retries(struct ofono_sim *sim)
 		nelem += 1;
 	}
 
-	ret = g_new0(void *, nelem * 2 + 1);
+	dict = g_new0(void *, nelem * 2 + 1);
+	retries = g_new0(unsigned char, nelem);
 
-	for (i = 1, nelem = 0; i < OFONO_SIM_PASSWORD_INVALID; i++) {
+	for (i = 1, nelem = 0; i < OFONO_SIM_PASSWORD_INVALID; i++, nelem++) {
 		if (sim->pin_retries[i] == -1)
 			continue;
 
-		ret[nelem++] = (void *) sim_passwd_name(i);
-		ret[nelem++] = &sim->pin_retries[i];
+		retries[nelem] = sim->pin_retries[i];
+
+		dict[nelem * 2] = (void *) sim_passwd_name(i);
+		dict[nelem * 2 + 1] = &retries[i];
 	}
 
-	return ret;
+	*out_dict = dict;
+	*out_retries = retries;
 }
 
 static char **get_service_numbers(GSList *service_numbers)
@@ -344,7 +350,8 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 	char **service_numbers;
 	char **locked_pins;
 	const char *pin_name;
-	void **pin_retries;
+	void **pin_retries_dict;
+	unsigned char *dbus_retries;
 	dbus_bool_t present = sim->state != OFONO_SIM_STATE_NOT_PRESENT;
 	dbus_bool_t fdn;
 	dbus_bool_t bdn;
@@ -419,10 +426,11 @@ static DBusMessage *sim_get_properties(DBusConnection *conn,
 				DBUS_TYPE_STRING,
 				(void *) &pin_name);
 
-	pin_retries = get_pin_retries(sim);
+	get_pin_retries(sim, &pin_retries_dict, &dbus_retries);
 	ofono_dbus_dict_append_dict(&dict, "Retries", DBUS_TYPE_BYTE,
-								&pin_retries);
-	g_free(pin_retries);
+							&pin_retries_dict);
+	g_free(pin_retries_dict);
+	g_free(dbus_retries);
 
 done:
 	dbus_message_iter_close_container(&iter, &dict);
@@ -437,7 +445,8 @@ static void sim_pin_retries_query_cb(const struct ofono_error *error,
 	struct ofono_sim *sim = data;
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
-	void **pin_retries;
+	void **pin_retries_dict;
+	unsigned char *dbus_retries;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		ofono_error("Querying remaining pin retries failed");
@@ -449,11 +458,12 @@ static void sim_pin_retries_query_cb(const struct ofono_error *error,
 
 	memcpy(sim->pin_retries, retries, sizeof(sim->pin_retries));
 
-	pin_retries = get_pin_retries(sim);
+	get_pin_retries(sim, &pin_retries_dict, &dbus_retries);
 	ofono_dbus_signal_dict_property_changed(conn, path,
 					OFONO_SIM_MANAGER_INTERFACE, "Retries",
-					DBUS_TYPE_BYTE,	&pin_retries);
-	g_free(pin_retries);
+					DBUS_TYPE_BYTE,	&pin_retries_dict);
+	g_free(pin_retries_dict);
+	g_free(dbus_retries);
 }
 
 static void sim_pin_retries_check(struct ofono_sim *sim)
-- 
2.0.4