[RFC PATCH v2 1/4] sms_util: status_report_expiration

[RFC PATCH v2 1/4] sms_util: status_report_expiration

[Petteri Tikander]

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

---
 src/smsutil.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++++---
 src/smsutil.h |    3 +--
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index 0de420b..ae8f1ef 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2961,10 +2961,55 @@ void status_report_assembly_add_fragment(
 }
 
 void status_report_assembly_expire(struct status_report_assembly *assembly,
-					time_t before, GFunc foreach_func,
-					gpointer data)
+					time_t before)
 {
-	/*TODO*/
+	GHashTable *id_table;
+	GHashTableIter iter_addr, iter_node;
+	struct sms_address addr;
+	char *straddr;
+	gpointer key;
+	unsigned int msg_id;
+	struct id_table_node *node;
+
+	g_hash_table_iter_init(&iter_addr, assembly->assembly_table);
+
+	/*
+	 * Go through different addresses. Each address can relate to
+	 * 1-n msg_ids.
+	 */
+	while (g_hash_table_iter_next(&iter_addr, (gpointer) &straddr,
+					(gpointer) &id_table)) {
+
+		sms_address_from_string(&addr, straddr);
+		g_hash_table_iter_init(&iter_node, id_table);
+
+		/* Go through different messages. */
+		while (g_hash_table_iter_next(&iter_node, &key,
+						(gpointer) &node)) {
+			msg_id = *(unsigned int *) key;
+
+			/*
+			 * If message is expired, removed it from the
+			 * hash-table and remove the backup-file
+			 */
+			if (node->expiration <= before) {
+				g_hash_table_iter_remove(&iter_node);
+
+				sr_assembly_remove_fragment_backup(
+								assembly->imsi,
+								node,
+								&addr,
+								msg_id);
+			}
+		}
+
+		/*
+		 * If all messages are removed, remove address
+		 * from the hash-table.
+		 */
+		if (g_hash_table_size(id_table) == 0)
+			g_hash_table_iter_remove(&iter_addr);
+	}
 }
 
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
diff --git a/src/smsutil.h b/src/smsutil.h
index 3c6b3ae..0e0ddf4 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -513,8 +513,7 @@ void status_report_assembly_add_fragment(struct status_report_assembly
 					unsigned char mr, time_t expiration,
 					unsigned char total_mrs);
 void status_report_assembly_expire(struct status_report_assembly *assembly,
-					time_t before, GFunc foreach_func,
-					gpointer data);
+					time_t before);
 
 GSList *sms_text_prepare(const char *utf8, guint16 ref,
 				gboolean use_16bit, int *ref_offset,
-- 
1.6.3.3

[RFC PATCH v2 2/4] unit: Add unit test for status report expiration

[Petteri Tikander]

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

---
 unit/test-sms.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/unit/test-sms.c b/unit/test-sms.c
index d56cdce..0ff9cd5 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -1301,6 +1301,13 @@ static void test_sr_assembly()
 	sms_address_from_string(&addr, "+4915259911630");
 
 	sra = status_report_assembly_new(NULL);
+
+	status_report_assembly_add_fragment(sra, 42, &addr, 4, time(NULL), 2);
+	status_report_assembly_add_fragment(sra, 42, &addr, 5, time(NULL), 2);
+
+	status_report_assembly_expire(sra, time(NULL) + 40);
+	g_assert(g_hash_table_size(sra->assembly_table) == 0);
+
 	status_report_assembly_add_fragment(sra, 42, &addr, 4, time(NULL), 2);
 	status_report_assembly_add_fragment(sra, 42, &addr, 5, time(NULL), 2);
 
-- 
1.6.3.3

[RFC PATCH v2 3/4] sms_util: remove unused parameter

[Petteri Tikander]

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

---
 src/smsutil.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index ae8f1ef..a124f54 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2771,7 +2771,6 @@ static gboolean sr_assembly_add_fragment_backup(const char *imsi,
 }
 
 static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
-					const struct id_table_node *node,
 					const struct sms_address *addr,
 					unsigned int msg_id)
 {
@@ -2901,7 +2900,7 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 	if (out_id)
 		*out_id = msg_id;
 
-	sr_assembly_remove_fragment_backup(assembly->imsi, node,
+	sr_assembly_remove_fragment_backup(assembly->imsi,
 					&status_report->status_report.raddr,
 					msg_id);
 
@@ -2997,7 +2996,6 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 
 				sr_assembly_remove_fragment_backup(
 								assembly->imsi,
-								node,
 								&addr,
 								msg_id);
 			}
-- 
1.6.3.3

[RFC PATCH v2 4/4] sms_util: fix status report backuping to support uint msg_id

[Petteri Tikander]

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

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

diff --git a/src/smsutil.c b/src/smsutil.c
index a124f54..26c7951 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -46,7 +46,7 @@
 #define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR "/%03i"
 
 #define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
-#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%i"
+#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%u"
 
 #define SMS_ADDR_FMT "%24[0-9A-F]"
 
@@ -2761,7 +2761,7 @@ static gboolean sr_assembly_add_fragment_backup(const char *imsi,
 	if (sms_address_to_hex_string(addr, straddr) == FALSE)
 		return FALSE;
 
-	/* storagedir/%s/sms_sr/%s-%i */
+	/* storagedir/%s/sms_sr/%s-%u */
 	if (write_file((unsigned char *) node, len, SMS_BACKUP_MODE,
 			SMS_SR_BACKUP_PATH_FILE, imsi,
 			straddr, msg_id) != len)
-- 
1.6.3.3

Re: [RFC PATCH v2 1/4] sms_util: status_report_expiration

[Denis Kenzior]

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

Hi Petteri,

On 09/10/2010 09:28 AM, Petteri Tikander wrote:
> ---
>  src/smsutil.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/smsutil.h |    3 +--
>  2 files changed, 49 insertions(+), 5 deletions(-)

All four patches have been applied.  Thanks :)

The only thing pending on this part is to handle the Status Report
recipient address being 'tweaked' by the network.

I've been hearing reports that some networks actually rewrite the
address (e.g. change national number to international, etc) which causes
our status reports to not match.

We can do two things:
1. Rely on message reference only.  This seems dangerous
2. Fallback to a 'fuzzy'.  E.g. try to match on the reference and last n
digits of the phone number, ignoring ton/npi.  This can get tricky as
some national numbers can get different prefixes from the equivalent
international number, so we need to pick a nice value of 'n'.

E.g. in Australia:

+61 4 1234 5678 is the international number or:
04 1234 5678 is the national number.

Let me know if you can think of a third option.

Regards,
-Denis

Re: [RFC PATCH v2 1/4] sms_util: status_report_expiration

[Petteri Tikander]

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

Hi Denis,

> Hi Petteri,
> 
> On 09/10/2010 09:28 AM, Petteri Tikander wrote:
> > ---
> >  src/smsutil.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++++---
> >  src/smsutil.h |    3 +--
> >  2 files changed, 49 insertions(+), 5 deletions(-)
> 
> All four patches have been applied.  Thanks :)

Thanks for you for assistance!

> 
> The only thing pending on this part is to handle the Status Report
> recipient address being 'tweaked' by the network.
> 
> I've been hearing reports that some networks actually rewrite the
> address (e.g. change national number to international, etc) which causes
> our status reports to not match.
> 
> We can do two things:
> 1. Rely on message reference only.  This seems dangerous
> 2. Fallback to a 'fuzzy'.  E.g. try to match on the reference and last n
> digits of the phone number, ignoring ton/npi.  This can get tricky as
> some national numbers can get different prefixes from the equivalent
> international number, so we need to pick a nice value of 'n'.
> 

Perhaps we can pick 'n' for number from the original number, in which SMS-
message was submitted:

      04 1234 5678 is the national number

Then we must remember to drop the first digit from the area code (or operator 
code or what ever it's called): 04=> 4 

So the number-string to be compared in the status report-response will be '4 
1234 5678'

And after receiving status report in the international format just find 
correspondence with the international number (+61 4 1234 5678); last digits 
should match.

OK. I don't know (at least haven't heart), will some operators drop more than 
one digit from the area code. This would cause problems anyway, when checking 
number-correspondence. But, I believe, number-checking presented above is 
better than nothing for 'national->international'-situation. 


> E.g. in Australia:
> 
> +61 4 1234 5678 is the international number or:
> 04 1234 5678 is the national number.
> 
> Let me know if you can think of a third option.
> 
> Regards,
> -Denis
> 

Have a nice week end.

Br, Petteri

Re: [RFC PATCH v2 1/4] sms_util: status_report_expiration

[Aki Niemi]

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

Hi Denis,

2010/9/10 Denis Kenzior <denkenz@gmail.com>:
> I've been hearing reports that some networks actually rewrite the
> address (e.g. change national number to international, etc) which causes
> our status reports to not match.

That happens.

> We can do two things:
> 1. Rely on message reference only.  This seems dangerous
> 2. Fallback to a 'fuzzy'.  E.g. try to match on the reference and last n
> digits of the phone number, ignoring ton/npi.  This can get tricky as
> some national numbers can get different prefixes from the equivalent
> international number, so we need to pick a nice value of 'n'.

I seem to recall some empirical evidence that n=6 works well, when
paired with the message reference.

Cheers,
Aki