Hi Petteri,
On 08/16/2010 02:08 AM, Petteri Tikander wrote:
> ---
> src/smsutil.c | 158
> +++++++++++++++++++++++++++++++++------------------------
> 1 files changed, 92 insertions(+), 66 deletions(-)
>
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 25e405c..a12bede 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -57,6 +57,15 @@ static GSList *sms_assembly_add_fragment_backup(struct
> sms_assembly *assembly,
> guint16 ref, guint8 max, guint8 seq,
> gboolean backup);
>
> +static void sr_assembly_add_fragment_backup(
> + struct status_report_assembly *assembly,
> + unsigned int msg_id, time_t ts,
> + const struct sms_address *to,
> + unsigned int *mrs,
> + unsigned char total_mrs,
> + unsigned char sent_mrs,
> + gboolean backup);
> +
Please avoid forward-declarations unless absolutely necessary. It is
better to just move the function upwards.
> /*
> * This function uses the meanings of digits 10..15 according to the rules
> * defined in 23.040 Section 9.1.2.3 and 24.008 Table 10.5.118
> @@ -2646,22 +2655,19 @@ void sms_assembly_expire(struct sms_assembly
> *assembly, time_t before)
> }
> }
>
> -static void sr_assembly_load_backup(GHashTable *assembly_table,
> - const char *imsi,
> - const struct dirent *addr_dir)
> +static void sr_assembly_load_backup(
> + struct status_report_assembly *assembly_table,
> + const struct dirent *addr_dir)
> {
> char *path;
> struct dirent **ids;
> struct sms_address addr;
> DECLARE_SMS_ADDR_STR(straddr);
> struct id_table_node *node;
> - GHashTable *id_table;
> int len;
> int r;
> unsigned char buf[sizeof(node->mrs) + sizeof(node->total_mrs) +
> sizeof(node->sent_mrs) + sizeof(node->deliverable)];
> - char *assembly_table_key;
> - unsigned int *id_table_key;
> struct stat segment_stat;
>
> if (addr_dir->d_type != DT_DIR)
> @@ -2675,7 +2681,7 @@ static void sr_assembly_load_backup(GHashTable
> *assembly_table,
> return;
>
> /* Go through different msg_ids. */
> - path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", imsi,
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", assembly_table->imsi,
> addr_dir->d_name);
> len = scandir(path, &ids, NULL, versionsort);
>
> @@ -2684,22 +2690,13 @@ static void sr_assembly_load_backup(GHashTable
> *assembly_table,
> if (len < 0)
> return;
>
> - id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> - g_free, g_free);
> -
> - assembly_table_key = g_try_malloc(sizeof(addr.address));
> -
> - if (assembly_table_key == NULL)
> - return;
> -
> - g_strlcpy(assembly_table_key, addr.address, sizeof(addr.address));
> - g_hash_table_insert(assembly_table, assembly_table_key, id_table);
> -
> while (len--) {
> path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s/%s",
> - imsi, addr_dir->d_name, ids[len]->d_name);
> + assembly_table->imsi, addr_dir->d_name,
> + ids[len]->d_name);
> r = read_file(buf, sizeof(buf), SMS_SR_BACKUP_PATH "/%s/%s",
> - imsi, addr_dir->d_name, ids[len]->d_name);
> + assembly_table->imsi, addr_dir->d_name,
> + ids[len]->d_name);
>
> if (r < 0) {
> g_free(path);
> @@ -2714,29 +2711,21 @@ static void sr_assembly_load_backup(GHashTable
> *assembly_table,
> g_free(ids[len]);
> continue;
> }
> - /* Gather the data for id_table node */
> - node = g_new0(struct id_table_node, 1);
> - memcpy(&node->to, &addr, sizeof(addr));
> - node->expiration = segment_stat.st_mtime;
> - memcpy(node->mrs, buf, sizeof(node->mrs));
> - memcpy(&node->total_mrs, buf + sizeof(node->mrs),
> - sizeof(node->total_mrs));
> - memcpy(&node->sent_mrs,
> - buf + sizeof(node->mrs) + sizeof(node->total_mrs),
> - sizeof(node->sent_mrs));
> -
> - memcpy(&node->deliverable, buf + sizeof(node->mrs) +
> - sizeof(node->total_mrs) + sizeof(node->sent_mrs),
> - sizeof(node->deliverable));
> - /* Node ready, create key and add them to the table */
> - id_table_key = g_new0(unsigned int, 1);
> - *id_table_key = atoi(ids[len]->d_name);
>
> - g_hash_table_insert(id_table, id_table_key, node);
> + sr_assembly_add_fragment_backup(assembly_table,
> + atoi(ids[len]->d_name),
> + segment_stat.st_mtime,
> + &addr,
> + (unsigned int *) &buf[0],
> + buf[sizeof(node->mrs)],
> + buf[sizeof(node->mrs) +
> + sizeof(node->total_mrs)],
> + FALSE);
>
If we can write / read the node structure directly, then I'm actually
thinking we won't be needing this patch at all.
> g_free(path);
> g_free(ids[len]);
> }
> +
> g_free(ids);
> }
>
> @@ -2767,8 +2756,7 @@ struct status_report_assembly
> *status_report_assembly_new(const char *imsi)
> * 1-n msg_ids.
> */
> while (len--) {
> - sr_assembly_load_backup(ret->assembly_table, imsi,
> - addresses[len]);
> + sr_assembly_load_backup(ret, addresses[len]);
> g_free(addresses[len]);
> }
>
> @@ -2778,16 +2766,17 @@ struct status_report_assembly
> *status_report_assembly_new(const char *imsi)
> return ret;
> }
>
> -static gboolean sr_assembly_add_fragment_backup(const char *imsi,
> - const struct id_table_node *node,
> - unsigned int msg_id)
> +static gboolean sr_assembly_backup_store(
> + struct status_report_assembly *assembly,
> + const struct id_table_node *node,
> + unsigned int msg_id)
> {
> int len = sizeof(node->mrs) + sizeof(node->total_mrs) +
> sizeof(node->sent_mrs) + sizeof(node->deliverable);
> unsigned char buf[len];
> DECLARE_SMS_ADDR_STR(straddr);
>
> - if (!imsi)
> + if (!assembly->imsi)
> return FALSE;
>
> if (sms_address_to_hex_string(&node->to, straddr) == FALSE)
> @@ -2805,32 +2794,35 @@ static gboolean sr_assembly_add_fragment_backup(const
> char *imsi,
> sizeof(node->sent_mrs), &node->deliverable, sizeof(node->deliverable));
>
> /* storagedir/%s/sms_sr/%s/%i */
> - if (write_file(buf, len, SMS_BACKUP_MODE, SMS_SR_BACKUP_PATH_FILE, imsi,
> + if (write_file(buf, len, SMS_BACKUP_MODE,
> + SMS_SR_BACKUP_PATH_FILE, assembly->imsi,
> straddr, msg_id) != len)
> return FALSE;
>
> return TRUE;
> }
>
> -static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
> +static gboolean sr_assembly_backup_free(struct status_report_assembly
> *assembly,
> const struct id_table_node *node,
> unsigned int msg_id)
> {
> char *path;
> DECLARE_SMS_ADDR_STR(straddr);
>
> - if (!imsi)
> + if (!assembly->imsi)
> return FALSE;
>
> if (sms_address_to_hex_string(&node->to, straddr) == FALSE)
> return FALSE;
>
> - path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, imsi, straddr, msg_id);
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, assembly->imsi,
> + straddr, msg_id);
>
> unlink(path);
> g_free(path);
>
> - path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, imsi, straddr);
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, assembly->imsi,
> + straddr);
>
> /* If the address does not have relating msg_ids anymore, remove it */
> rmdir(path);
> @@ -2928,8 +2920,8 @@ gboolean status_report_assembly_report(struct
> status_report_assembly *assembly,
> /* More status reports expected, and already received
> reports completed. Update backup file.
> */
> - sr_assembly_add_fragment_backup(assembly->imsi, node,
> - *((unsigned int *) key));
> + sr_assembly_backup_store(assembly, node,
> + *((unsigned int *) key));
>
> return FALSE;
> }
> @@ -2940,8 +2932,7 @@ gboolean status_report_assembly_report(struct
> status_report_assembly *assembly,
> if (out_id)
> *out_id = *((unsigned int *) key);
>
> - sr_assembly_remove_fragment_backup(assembly->imsi, node,
> - *((unsigned int *) key));
> + sr_assembly_backup_free(assembly, node, *((unsigned int *) key));
>
> g_hash_table_iter_remove(&iter);
>
> @@ -2952,18 +2943,18 @@ gboolean status_report_assembly_report(struct
> status_report_assembly *assembly,
> return TRUE;
> }
>
> -void status_report_assembly_add_fragment(
> - struct status_report_assembly *assembly,
> - unsigned int msg_id,
> - const struct sms_address *to,
> - unsigned char mr, time_t expiration,
> - unsigned char total_mrs)
> +static void sr_assembly_add_fragment_backup(
> + struct status_report_assembly *assembly,
> + unsigned int msg_id, time_t ts,
> + const struct sms_address *to,
> + unsigned int *mrs, unsigned char total_mrs,
> + unsigned char sent_mrs, gboolean backup)
> {
> - unsigned int offset = mr / 32;
> - unsigned int bit = 1 << (mr % 32);
> GHashTable *id_table;
> struct id_table_node *node;
> unsigned int *id_table_key;
> + int i;
> + int len = sizeof(node->mrs) / sizeof(node->mrs[0]);
>
> id_table = g_hash_table_lookup(assembly->assembly_table,
> sms_address_to_string(to));
> @@ -2971,7 +2962,7 @@ void status_report_assembly_add_fragment(
> /* Create hashtable keyed by the to address if required */
> if (id_table == NULL) {
> id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> - g_free, g_free);
> + g_free, g_free);
> g_hash_table_insert(assembly->assembly_table,
> g_strdup(sms_address_to_string(to)),
> id_table);
> @@ -2993,10 +2984,45 @@ void status_report_assembly_add_fragment(
> }
>
> /* id_table and node both exists */
> - node->mrs[offset] |= bit;
> - node->expiration = expiration;
> - node->sent_mrs++;
> - sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
> +
> + for (i = 0; i < len; i++)
> + node->mrs[i] |= mrs[i];
> +
> + node->expiration = ts;
> +
> + /* storing to the backup-file is needed, when new SMS-message is sent */
> + if (backup) {
> + node->sent_mrs++;
> + sr_assembly_backup_store(assembly, node, msg_id);
> + }
> + /* storing to the backup-file is not needed, when backup is loaded */
> + else
> + node->sent_mrs = sent_mrs;
> +}
> +
> +void status_report_assembly_add_fragment(
> + struct status_report_assembly *assembly,
> + unsigned int msg_id,
> + const struct sms_address *to,
> + unsigned char mr, time_t expiration,
> + unsigned char total_mrs)
> +{
> + struct id_table_node *node;
> + unsigned int offset = mr / 32;
> + unsigned int bit = 1 << (mr % 32);
> + int len = sizeof(node->mrs) / sizeof(node->mrs[0]);
> + unsigned int mrs[len];
> +
> + memset(mrs, 0, len);
> +
> + /* set correct bit corresponding to the
> + * received mr-number in mr-buffer
> + */
> + mrs[offset] |= bit;
> +
The memset above and this looks fishy to me...
> + sr_assembly_add_fragment_backup(assembly, msg_id, expiration,
> + to, mrs, total_mrs,
> + FALSE, TRUE);
> }
>
> void status_report_assembly_expire(struct status_report_assembly *assembly,
Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
