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.

diff --git a/src/sim.c b/src/sim.c
index eb50d8b..ab38e4f 100644 (file)
--- 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)