[PATCH 0/4] Icon patches (rebased)

[PATCH 0/4] Icon patches (rebased)

[Kristen Carlson Accardi]

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

Rebased old patch.  Found some errors that had been introduced into the
simfs code.  Added file type to simutil, although I couldn't test it
on any real hardware.

Kristen Carlson Accardi (4):
  simfs: cache images
  sim: implement GetIcon
  simfs: fix incorrect math when calculating length to copy
  simutil: add file type for EFimg

 src/sim.c     |  187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/simfs.c   |   83 ++++++++++++++++++++++---
 src/simfs.h   |    4 +
 src/simutil.c |    1 +
 4 files changed, 266 insertions(+), 9 deletions(-)

-- 
1.7.2.1

[PATCH 1/4] simfs: cache images

[Kristen Carlson Accardi]

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

---
 src/simfs.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/simfs.h |    4 +++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index b5b12b4..5fbee8b 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -47,6 +47,7 @@
 #define SIM_CACHE_PATH SIM_CACHE_BASEPATH "/%04x"
 #define SIM_CACHE_HEADER_SIZE 38
 #define SIM_FILE_INFO_SIZE 6
+#define SIM_IMAGE_CACHE_PATH STORAGEDIR "/%s-%i/images/%d.xpm"
 
 #define SIM_FS_VERSION 1
 
@@ -722,6 +723,67 @@ int sim_fs_write(struct sim_fs *fs, int id, ofono_sim_file_write_cb_t cb,
 	return 0;
 }
 
+void sim_fs_cache_image(struct sim_fs *fs, const char *image, int id)
+{
+	const char *imsi;
+	enum ofono_sim_phase phase;
+
+	if (fs == NULL || image == NULL)
+		return;
+
+	imsi = ofono_sim_get_imsi(fs->sim);
+	phase = ofono_sim_get_phase(fs->sim);
+
+	if (imsi)
+		write_file((const unsigned char *) image, strlen(image),
+				SIM_CACHE_MODE, SIM_IMAGE_CACHE_PATH, imsi,
+				phase, id);
+}
+
+char *sim_fs_get_cached_image(struct sim_fs *fs, int id)
+{
+	const char *imsi;
+	enum ofono_sim_phase phase;
+	unsigned short image_length;
+	int fd;
+	char *buffer;
+	char *path;
+	int len;
+	struct stat st_buf;
+
+	if (fs == NULL)
+		return NULL;
+
+	imsi = ofono_sim_get_imsi(fs->sim);
+	phase = ofono_sim_get_phase(fs->sim);
+
+	path = g_strdup_printf(SIM_IMAGE_CACHE_PATH, imsi, phase, id);
+
+	TFR(stat(path, &st_buf));
+
+	fd = TFR(open(path, O_RDONLY));
+
+	g_free(path);
+
+	if (fd < 0)
+		return NULL;
+
+	image_length = st_buf.st_size;
+
+	buffer = g_try_malloc0(image_length + 1);
+
+	len = TFR(read(fd, buffer, image_length));
+
+	TFR(close(fd));
+
+	if (len != image_length) {
+		g_free(buffer);
+		return NULL;
+	}
+
+	return buffer;
+}
+
 static void remove_cachefile(const char *imsi, enum ofono_sim_phase phase,
 				const struct dirent *file)
 {
diff --git a/src/simfs.h b/src/simfs.h
index 6d5dded..c0b4c51 100644
--- a/src/simfs.h
+++ b/src/simfs.h
@@ -35,4 +35,8 @@ int sim_fs_write(struct sim_fs *fs, int id, ofono_sim_file_write_cb_t cb,
 			enum ofono_sim_file_structure structure, int record,
 			const unsigned char *data, int length, void *userdata);
 
+char *sim_fs_get_cached_image(struct sim_fs *fs, int id);
+
+void sim_fs_cache_image(struct sim_fs *fs, const char *image, int id);
+
 void sim_fs_free(struct sim_fs *fs);
-- 
1.7.2.1

Re: [PATCH 1/4] simfs: cache images

[Denis Kenzior]

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

Hi Kristen,

On 10/04/2010 05:33 PM, Kristen Carlson Accardi wrote:
> ---
>  src/simfs.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/simfs.h |    4 +++
>  2 files changed, 66 insertions(+), 0 deletions(-)

Patch has been applied with some minor fixes afterwards.

Regards,
-Denis

[PATCH 2/4] sim: implement GetIcon

[Kristen Carlson Accardi]

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

---
 src/sim.c |  187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 187 insertions(+), 0 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 6f10d4c..a671a8d 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -43,6 +43,7 @@
 #include "simutil.h"
 #include "storage.h"
 #include "simfs.h"
+#include "stkutil.h"
 
 static GSList *g_drivers = NULL;
 
@@ -721,6 +722,190 @@ 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)
+{
+	struct ofono_sim *sim = userdata;
+	DBusMessage *reply;
+
+	if (!ok)
+		reply = __ofono_error_failed(sim->pending);
+	else {
+		reply = dbus_message_new_method_return(sim->pending);
+		dbus_message_append_args(reply, DBUS_TYPE_STRING, &xpm,
+					DBUS_TYPE_INVALID);
+	}
+
+	__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;
+};
+
+static void sim_iidf_read_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;
+	char *xpm;
+	struct ofono_sim *sim = image->sim;
+
+	if (!ok) {
+		sim_get_image_cb(ok, NULL, 0, image->user_data);
+		goto iidf_read_out;
+	}
+
+	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);
+		}
+
+		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);
+
+		sim_get_image_cb(ok, xpm, strlen(xpm), image->user_data);
+
+		g_free(xpm);
+
+		goto iidf_read_out;
+	}
+
+	offset = data[4] << 8 | data[5];
+	num_entries = data[3];
+
+	if (num_entries == 0)
+		num_entries = 256;
+
+	/* indicate that we're on our second read */
+	image->need_clut = FALSE;
+
+	image->clut_len = num_entries * 3;
+
+	image->image = g_memdup(data, length);
+
+	/* read the clut data */
+	ofono_sim_read_bytes(image->sim, image->iidf_id,
+				offset, image->clut_len,
+				sim_iidf_read_cb, image);
+
+	return;
+
+iidf_read_out:
+	g_free(image->image);
+	g_free(image);
+}
+
+static void sim_get_image(struct ofono_sim *sim, unsigned char id,
+			gpointer user_data)
+{
+	struct image_data *data;
+	unsigned char *efimg;
+	char *image;
+
+	/* icon ids should start at 1, our array starts at zero */
+	if (id == 0) {
+		sim_get_image_cb(0, NULL, 0, user_data);
+		return;
+	}
+
+	id--;
+
+	image = sim_fs_get_cached_image(sim->simfs, id);
+
+	if (image == NULL)
+		goto read_image;
+
+	sim_get_image_cb(1, image, strlen(image), user_data);
+
+	g_free(image);
+
+	return;
+
+read_image:
+
+	if (sim->efimg_length <= (id * 9)) {
+		sim_get_image_cb(0, NULL, 0, user_data);
+		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;
+
+	/* read the image data */
+	ofono_sim_read_bytes(sim, data->iidf_id,
+				data->iidf_offset, data->iid_len,
+				sim_iidf_read_cb, data);
+}
+
+static DBusMessage *sim_get_icon(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_sim *sim = data;
+	unsigned char id;
+
+	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_BYTE, &id,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	/* zero means no icon */
+	if (id == 0)
+		return __ofono_error_invalid_args(msg);
+
+	if (sim->pending)
+		return __ofono_error_busy(msg);
+
+	if (sim->efimg == NULL)
+		return __ofono_error_not_implemented(msg);
+
+	sim->pending = dbus_message_ref(msg);
+
+	sim_get_image(sim, id, sim);
+
+	return NULL;
+}
+
 static DBusMessage *sim_reset_pin(DBusConnection *conn, DBusMessage *msg,
 					void *data)
 {
@@ -773,6 +958,8 @@ static GDBusMethodTable sim_methods[] = {
 							G_DBUS_METHOD_FLAG_ASYNC },
 	{ "UnlockPin",		"ss",	"",		sim_unlock_pin,
 							G_DBUS_METHOD_FLAG_ASYNC },
+	{ "GetIcon",		"y",	"s",		sim_get_icon,
+							G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
-- 
1.7.2.1

Re: [PATCH 2/4] sim: implement GetIcon

[Denis Kenzior]

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

Hi Kristen,

On 10/04/2010 05:33 PM, Kristen Carlson Accardi wrote:
> ---
>  src/sim.c |  187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 187 insertions(+), 0 deletions(-)
> 

I applied this patch but refactored it afterwards to avoid a potential
memory leak.  Can you please check that I haven't broken anything?

Thanks,
-Denis

[PATCH 3/4] simfs: fix incorrect math when calculating length to copy

[Kristen Carlson Accardi]

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

---
 src/simfs.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index 5fbee8b..bfcdf23 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -229,14 +229,16 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
 	if (op->current == start_block) {
 		bufoff = 0;
 		dataoff = op->offset % 256;
-		tocopy = MIN(256 - op->offset % 256, len);
 	} else {
-		bufoff = (op->current - start_block - 1) * 256 +
-				op->offset % 256;
+		bufoff = op->current * 256;
 		dataoff = 0;
-		tocopy = len;
 	}
 
+	tocopy = op->num_bytes % 256;
+
+	if ((tocopy == 0) || (op->current != end_block))
+		tocopy = 256;
+
 	memcpy(fs->buffer + bufoff, data + dataoff, tocopy);
 	cache_block(fs, op->current, 256, data, len);
 
@@ -288,15 +290,16 @@ static gboolean sim_fs_op_read_block(gpointer user_data)
 			bufoff = 0;
 			seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256 +
 				op->offset % 256;
-			toread = MIN(256 - op->offset % 256,
-					op->length - op->current * 256);
 		} else {
-			bufoff = (op->current - start_block - 1) * 256 +
-					op->offset % 256;
+			bufoff = op->current * 256;
 			seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256;
-			toread = MIN(256, op->length - op->current * 256);
 		}
 
+		toread = op->num_bytes % 256;
+
+		if ((toread == 0) || (op->current != end_block))
+			toread = 256;
+
 		if (lseek(fs->fd, seekoff, SEEK_SET) == (off_t) -1)
 			break;
 
-- 
1.7.2.1

Re: [PATCH 3/4] simfs: fix incorrect math when calculating length to copy

[Denis Kenzior]

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

Hi Kristen,

On 10/04/2010 05:33 PM, Kristen Carlson Accardi wrote:
> ---
>  src/simfs.c |   21 ++++++++++++---------
>  1 files changed, 12 insertions(+), 9 deletions(-)
> 

I fixed this slightly differently, can you check that my fix is correct?

Regards,
-Denis

[PATCH] simfs: fix incorrect math again

[Kristen Carlson Accardi]

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

---
 src/simfs.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index fd768ef..bcb6ff8 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -231,8 +231,7 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
 		tocopy = MIN(256 - op->offset % 256,
 				op->num_bytes - op->current * 256);
 	} else {
-		bufoff = (op->current - start_block - 1) * 256 +
-				op->offset % 256;
+		bufoff = op->current * 256;
 		dataoff = 0;
 		tocopy = MIN(256, op->num_bytes - op->current * 256);
 	}
@@ -294,8 +293,7 @@ static gboolean sim_fs_op_read_block(gpointer user_data)
 			toread = MIN(256 - op->offset % 256,
 					op->num_bytes - op->current * 256);
 		} else {
-			bufoff = (op->current - start_block - 1) * 256 +
-					op->offset % 256;
+			bufoff = op->current * 256;
 			seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256;
 			toread = MIN(256, op->num_bytes - op->current * 256);
 		}
-- 
1.7.2.3

Re: [PATCH] simfs: fix incorrect math again

[Kristen Carlson Accardi]

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

On Thu, 14 Oct 2010 11:18:00 -0700
Kristen Carlson Accardi <kristen@linux.intel.com> wrote:

> ---
>  src/simfs.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/simfs.c b/src/simfs.c
> index fd768ef..bcb6ff8 100644
> --- a/src/simfs.c
> +++ b/src/simfs.c
> @@ -231,8 +231,7 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
>  		tocopy = MIN(256 - op->offset % 256,
>  				op->num_bytes - op->current * 256);
>  	} else {
> -		bufoff = (op->current - start_block - 1) * 256 +
> -				op->offset % 256;
> +		bufoff = op->current * 256;
>  		dataoff = 0;
>  		tocopy = MIN(256, op->num_bytes - op->current * 256);
>  	}
> @@ -294,8 +293,7 @@ static gboolean sim_fs_op_read_block(gpointer user_data)
>  			toread = MIN(256 - op->offset % 256,
>  					op->num_bytes - op->current * 256);
>  		} else {
> -			bufoff = (op->current - start_block - 1) * 256 +
> -					op->offset % 256;
> +			bufoff = op->current * 256;
>  			seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256;
>  			toread = MIN(256, op->num_bytes - op->current * 256);
>  		}

Denis - any feedback on this patch?  If we don't add this bugfix,
reads > 256 bytes will be broken.

Thanks,
Kristen

Re: [PATCH] simfs: fix incorrect math again

[Denis Kenzior]

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

Hi Kristen,

On 11/02/2010 09:41 AM, Kristen Carlson Accardi wrote:
> On Thu, 14 Oct 2010 11:18:00 -0700
> Kristen Carlson Accardi <kristen@linux.intel.com> wrote:
> 
>> ---
>>  src/simfs.c |    6 ++----
>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/simfs.c b/src/simfs.c
>> index fd768ef..bcb6ff8 100644
>> --- a/src/simfs.c
>> +++ b/src/simfs.c
>> @@ -231,8 +231,7 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
>>  		tocopy = MIN(256 - op->offset % 256,
>>  				op->num_bytes - op->current * 256);
>>  	} else {
>> -		bufoff = (op->current - start_block - 1) * 256 +
>> -				op->offset % 256;
>> +		bufoff = op->current * 256;
>>  		dataoff = 0;
>>  		tocopy = MIN(256, op->num_bytes - op->current * 256);
>>  	}
>> @@ -294,8 +293,7 @@ static gboolean sim_fs_op_read_block(gpointer user_data)
>>  			toread = MIN(256 - op->offset % 256,
>>  					op->num_bytes - op->current * 256);
>>  		} else {
>> -			bufoff = (op->current - start_block - 1) * 256 +
>> -					op->offset % 256;
>> +			bufoff = op->current * 256;
>>  			seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256;
>>  			toread = MIN(256, op->num_bytes - op->current * 256);
>>  		}
> 
> Denis - any feedback on this patch?  If we don't add this bugfix,
> reads > 256 bytes will be broken.
> 

I don't believe the patch is correct:

In sim_fs_op_read_block:
...
        if (op->current == start_block) {
                op->buffer = g_try_new0(unsigned char, op->num_bytes);
...

We don't allocate a buffer for the entire file, just for the portion
we're reading.  So setting buffoff to op->current * 256 will not work on
reads that start at non-zero offsets.

Regards,
-Denis

[PATCH] simfs: make sim_fs_read_block work for > 256 byte files

[Kristen Carlson Accardi]

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

---
 src/simfs.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index 5f459ab..258a42f 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -59,6 +59,7 @@ static gboolean sim_fs_op_read_block(gpointer user_data);
 struct sim_fs_op {
 	int id;
 	unsigned char *buffer;
+	int bufoff;
 	enum ofono_sim_file_structure structure;
 	unsigned short offset;
 	gboolean info_only;
@@ -217,7 +218,6 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
 	struct sim_fs_op *op = g_queue_peek_head(fs->op_q);
 	int start_block;
 	int end_block;
-	int bufoff;
 	int dataoff;
 	int tocopy;
 
@@ -230,25 +230,25 @@ static void sim_fs_op_read_block_cb(const struct ofono_error *error,
 	end_block = (op->offset + (op->num_bytes - 1)) / 256;
 
 	if (op->current == start_block) {
-		bufoff = 0;
+		op->bufoff = 0;
 		dataoff = op->offset % 256;
 		tocopy = MIN(256 - op->offset % 256,
 				op->num_bytes - op->current * 256);
 	} else {
-		bufoff = (op->current - start_block - 1) * 256 +
-				op->offset % 256;
 		dataoff = 0;
 		tocopy = MIN(256, op->num_bytes - op->current * 256);
 	}
 
 	DBG("bufoff: %d, dataoff: %d, tocopy: %d",
-				bufoff, dataoff, tocopy);
+				op->bufoff, dataoff, tocopy);
 
-	memcpy(op->buffer + bufoff, data + dataoff, tocopy);
+	memcpy(op->buffer + op->bufoff, data + dataoff, tocopy);
 	cache_block(fs, op->current, 256, data, len);
 
 	op->current++;
 
+	op->bufoff += tocopy;
+
 	if (op->current > end_block) {
 		ofono_sim_file_read_cb_t cb = op->cb;
 
-- 
1.7.2.3

[PATCH 4/4] simutil: add file type for EFimg

[Kristen Carlson Accardi]

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

---
 src/simutil.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/simutil.c b/src/simutil.c
index 65ce1ac..ed0970c 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -69,6 +69,7 @@ static struct sim_ef_info ef_db[] = {
 {	0x2F05, ROOTMF, BINARY, 0,	ALW,	PIN	},
 {	0x2F06, ROOTMF, RECORD, 0,	ALW,	PIN	},
 {	0x2FE2, ROOTMF, BINARY, 10,	ALW,	NEV 	},
+{	0x4F20, 0x5F50, BINARY, 0,	PIN,	ADM	},
 {	0x6F05, 0x7F20, BINARY, 0,	ALW,	PIN	},
 {	0x6F06, 0x0000, RECORD, 0,	ALW,	ADM	},
 {	0x6F2C, 0x7F20, BINARY, 16,	PIN,	PIN	},
-- 
1.7.2.1

Re: [PATCH 4/4] simutil: add file type for EFimg

[Denis Kenzior]

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

Hi Kristen,

On 10/04/2010 05:33 PM, Kristen Carlson Accardi wrote:
> ---
>  src/simutil.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis