sim: Refactor iidf reading

Get rid of image_data since it can lead to potential memory leaks when
sim is removed while the IIDF operations are in progress.

1 files changed, 90 insertions, 90 deletions

diff --git a/src/sim.c b/src/sim.c
index eb50d8b8..ab38e4f5 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -91,6 +91,8 @@ struct ofono_sim {
 
 	struct sim_fs *simfs;
 
+	unsigned char *iidf_image;
+
 	DBusMessage *pending;
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
@@ -722,19 +724,21 @@ static DBusMessage *sim_enter_pin(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
-static void sim_get_image_cb(gboolean ok, const char *xpm, int xpm_len,
-					void *userdata)
+static void sim_get_image_cb(struct ofono_sim *sim,
+				unsigned char id, char *xpm, gboolean cache)
 {
-	struct ofono_sim *sim = userdata;
 	DBusMessage *reply;
 	DBusMessageIter iter, array;
+	int xpm_len;
 
-	if (ok == FALSE) {
+	if (xpm == NULL) {
 		reply = __ofono_error_failed(sim->pending);
 		__ofono_dbus_pending_reply(&sim->pending, reply);
 		return;
 	}
 
+	xpm_len = strlen(xpm);
+
 	reply = dbus_message_new_method_return(sim->pending);
 	dbus_message_iter_init_append(reply, &iter);
 
@@ -746,133 +750,126 @@ static void sim_get_image_cb(gboolean ok, const char *xpm, int xpm_len,
 	dbus_message_iter_close_container(&iter, &array);
 
 	__ofono_dbus_pending_reply(&sim->pending, reply);
-}
 
-struct image_data {
-	struct ofono_sim *sim;
-	unsigned char width;
-	unsigned char height;
-	enum stk_img_scheme scheme;
-	unsigned short iidf_id;
-	unsigned short iidf_offset;
-	unsigned short iid_len;
-	void *image;
-	unsigned short clut_len;
-	gboolean need_clut;
-	gpointer user_data;
-	unsigned char id;
-};
+	if (cache)
+		sim_fs_cache_image(sim->simfs, (const char *) xpm, id);
 
-static void sim_iidf_read_cb(int ok, int length, int record,
-				const unsigned char *data,
-				int record_length, void *userdata)
+	g_free(xpm);
+}
+
+static void sim_iidf_read_clut_cb(int ok, int length, int record,
+					const unsigned char *data,
+					int record_length, void *userdata)
 {
-	struct image_data *image = userdata;
-	unsigned short offset;
-	unsigned short num_entries;
+	struct ofono_sim *sim = userdata;
+	unsigned char id;
+	unsigned char *efimg;
+	unsigned short iidf_len;
+	unsigned short clut_len;
 	char *xpm;
-	struct ofono_sim *sim = image->sim;
+
+	DBG("ok: %d", ok);
+
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_BYTE, &id,
+					DBUS_TYPE_INVALID);
+	id -= 1;
+	efimg = &sim->efimg[id * 9];
 
 	if (!ok) {
-		sim_get_image_cb(ok, NULL, 0, image->user_data);
-		goto iidf_read_out;
+		sim_get_image_cb(sim, id, NULL, FALSE);
+		goto done;
 	}
 
-	if (image->need_clut == FALSE) {
-		if (image->scheme == STK_IMG_SCHEME_BASIC) {
-			xpm = stk_image_to_xpm(data, image->iid_len,
-						image->scheme, NULL, 0);
-		} else {
-			xpm = stk_image_to_xpm(image->image, image->iid_len,
-						image->scheme, data,
-						image->clut_len);
-		}
+	iidf_len = efimg[7] << 8 | efimg[8];
 
-		if (xpm == NULL) {
-			sim_get_image_cb(0, NULL, 0, image->user_data);
-			goto iidf_read_out;
-		}
-
-		sim_fs_cache_image(sim->simfs, (const char *) xpm,
-					image->id);
+	if (sim->iidf_image[3] == 0)
+		clut_len = 256 * 3;
+	else
+		clut_len = sim->iidf_image[3] * 3;
 
-		sim_get_image_cb(ok, xpm, strlen(xpm), image->user_data);
+	xpm = stk_image_to_xpm(sim->iidf_image, iidf_len, efimg[2],
+					data, clut_len);
+	sim_get_image_cb(sim, id, xpm, TRUE);
 
-		g_free(xpm);
+done:
+	g_free(sim->iidf_image);
+	sim->iidf_image = NULL;
+}
 
-		goto iidf_read_out;
-	}
+static void sim_iidf_read_cb(int ok, int length, int record,
+				const unsigned char *data,
+				int record_length, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+	unsigned char id;
+	unsigned char *efimg;
+	unsigned short iidf_id;
+	unsigned short offset;
+	unsigned short clut_len;
 
-	offset = data[4] << 8 | data[5];
-	num_entries = data[3];
+	DBG("ok: %d", ok);
 
-	if (num_entries == 0)
-		num_entries = 256;
+	dbus_message_get_args(sim->pending, NULL, DBUS_TYPE_BYTE, &id,
+					DBUS_TYPE_INVALID);
+	id -= 1;
+	efimg = &sim->efimg[id * 9];
 
-	/* indicate that we're on our second read */
-	image->need_clut = FALSE;
+	if (!ok) {
+		sim_get_image_cb(sim, id, NULL, FALSE);
+		return;
+	}
 
-	image->clut_len = num_entries * 3;
+	if (efimg[2] == STK_IMG_SCHEME_BASIC) {
+		char *xpm = stk_image_to_xpm(data, length, efimg[2], NULL, 0);
+		sim_get_image_cb(sim, id, xpm, TRUE);
+		return;
+	}
 
-	image->image = g_memdup(data, length);
+	offset = data[4] << 8 | data[5];
 
-	/* read the clut data */
-	ofono_sim_read_bytes(image->sim, image->iidf_id,
-				offset, image->clut_len,
-				sim_iidf_read_cb, image);
+	if (data[3] == 0)
+		clut_len = 256 * 3;
+	else
+		clut_len = data[3] * 3;
 
-	return;
+	iidf_id = efimg[3] << 8 | efimg[4];
+	sim->iidf_image = g_memdup(data, length);
 
-iidf_read_out:
-	g_free(image->image);
-	g_free(image);
+	/* read the clut data */
+	ofono_sim_read_bytes(sim, iidf_id, offset, clut_len,
+					sim_iidf_read_clut_cb, sim);
 }
 
 static void sim_get_image(struct ofono_sim *sim, unsigned char id,
-			gpointer user_data)
+				gpointer user_data)
 {
-	struct image_data *data;
 	unsigned char *efimg;
 	char *image;
+	unsigned short iidf_id;
+	unsigned short iidf_offset;
+	unsigned short iidf_len;
 
 	image = sim_fs_get_cached_image(sim->simfs, id);
 
 	if (image != NULL) {
-		sim_get_image_cb(1, image, strlen(image), user_data);
-		g_free(image);
+		sim_get_image_cb(sim, id, image, FALSE);
 		return;
 	}
 
 	if (sim->efimg_length <= (id * 9)) {
-		sim_get_image_cb(0, NULL, 0, user_data);
+		sim_get_image_cb(sim, id, NULL, FALSE);
 		return;
 	}
 
 	efimg = &sim->efimg[id * 9];
 
-	data = g_try_new0(struct image_data, 1);
-	if (data == NULL)
-		return;
-
-	data->width = efimg[0];
-	data->height = efimg[1];
-	data->scheme = efimg[2];
-	data->iidf_id = efimg[3] << 8 | efimg[4];
-	data->iidf_offset = efimg[5] << 8 | efimg[6];
-	data->iid_len = efimg[7] << 8 | efimg[8];
-	data->user_data = user_data;
-	data->sim = sim;
-	data->id = id;
-
-	if (data->scheme == STK_IMG_SCHEME_BASIC)
-		data->need_clut = FALSE;
-	else
-		data->need_clut = TRUE;
+	iidf_id = efimg[3] << 8 | efimg[4];
+	iidf_offset = efimg[5] << 8 | efimg[6];
+	iidf_len = efimg[7] << 8 | efimg[8];
 
 	/* read the image data */
-	ofono_sim_read_bytes(sim, data->iidf_id,
-				data->iidf_offset, data->iid_len,
-				sim_iidf_read_cb, data);
+	ofono_sim_read_bytes(sim, iidf_id, iidf_offset, iidf_len,
+				sim_iidf_read_cb, sim);
 }
 
 static DBusMessage *sim_get_icon(DBusConnection *conn,
@@ -1848,6 +1845,9 @@ static void sim_free_state(struct ofono_sim *sim)
 		sim->efimg = NULL;
 		sim->efimg_length = 0;
 	}
+
+	g_free(sim->iidf_image);
+	sim->iidf_image = NULL;
 }
 
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)