All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] smsutil: national to international conversion in status report
@ 2010-09-14 17:38 Petteri Tikander
  2010-09-14 17:38 ` [RFC PATCH 2/2] unit: national to international conversion Petteri Tikander
  0 siblings, 1 reply; 6+ messages in thread
From: Petteri Tikander @ 2010-09-14 17:38 UTC (permalink / raw)
  To: ofono

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

---
 src/smsutil.c |   70 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index 26c7951..2d47289 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2826,14 +2826,18 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 	unsigned int offset = status_report->status_report.mr / 32;
 	unsigned int bit = 1 << (status_report->status_report.mr % 32);
 	struct id_table_node *node = NULL;
-	const char *straddr;
+	const char *straddr, *sent_addr;
+	struct sms_address addr;
 	GHashTable *id_table;
 	gpointer key, value;
 	gboolean delivered;
-	GHashTableIter iter;
+	GHashTableIter iter_addr, iter;
 	gboolean pending;
 	int i;
 	unsigned int msg_id;
+	unsigned int n_digits;
+	unsigned int len_sent_addr;
+	unsigned int len_rec_addr;
 
 	/* We ignore temporary or tempfinal status reports */
 	if (sr_st_to_delivered(status_report->status_report.st,
@@ -2841,20 +2845,54 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		return FALSE;
 
 	straddr = sms_address_to_string(&status_report->status_report.raddr);
-	id_table = g_hash_table_lookup(assembly->assembly_table, straddr);
 
-	/* key (receiver address) does not exist in assembly */
-	if (id_table == NULL)
-		return FALSE;
+	g_hash_table_iter_init(&iter_addr, assembly->assembly_table);
 
-	g_hash_table_iter_init(&iter, id_table);
-	while (g_hash_table_iter_next(&iter, &key, &value)) {
-		node = value;
+	/*
+	 * Go through all addresses. Each address can relate to
+	 * 1-n msg_ids.
+	 */
+	while (g_hash_table_iter_next(&iter_addr, (gpointer) &sent_addr,
+					(gpointer) &id_table)) {
+		/*
+		 * Some networks can change address to international format,
+		 * although address is sent in the national format.
+		 * So notify this special case by checking only
+		 * last six digits. If address contains less than six digits,
+		 * compare only existing digits.
+		 */
+		if ((straddr[0] == '+') && (sent_addr[0] != '+')) {
+			len_sent_addr = strlen(sent_addr);
+			len_rec_addr = strlen(straddr);
+			n_digits = (len_sent_addr > 6) ? 6 : len_sent_addr;
 
-		if (node->mrs[offset] & bit)
-			break;
+			if (strcmp(&straddr[len_rec_addr - n_digits],
+					&sent_addr[len_sent_addr - n_digits]))
+				continue;
+		}
+		/*
+		 * In other cases the whole number received in the status report
+		 * should match with the number originally sent.
+		 */
+		else if (strcmp(straddr, sent_addr))
+				continue;
+
+		g_hash_table_iter_init(&iter, id_table);
+		while (g_hash_table_iter_next(&iter, &key, &value)) {
+			node = value;
+
+			if (node->mrs[offset] & bit)
+				break;
 
-		node = NULL;
+			node = NULL;
+		}
+
+		/*
+		 * Received address with MR matched with one of the stored
+		 * addresses and MR, so no need to continue searching.
+		 */
+		if (node)
+			break;
 	}
 
 	/* Unable to find a message reference belonging to this address */
@@ -2881,6 +2919,8 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 
 	msg_id = *(unsigned int *) key;
 
+	sms_address_from_string(&addr, sent_addr);
+
 	if (pending == TRUE && node->deliverable == TRUE) {
 		/*
 		 * More status reports expected, and already received
@@ -2888,7 +2928,7 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		 */
 		sr_assembly_add_fragment_backup(
 					assembly->imsi, node,
-					&status_report->status_report.raddr,
+					&addr,
 					msg_id);
 
 		return FALSE;
@@ -2901,14 +2941,14 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		*out_id = msg_id;
 
 	sr_assembly_remove_fragment_backup(assembly->imsi,
-					&status_report->status_report.raddr,
+					&addr,
 					msg_id);
 
 	g_hash_table_iter_remove(&iter);
 
 	if (g_hash_table_size(id_table) == 0)
 		g_hash_table_remove(assembly->assembly_table,
-				status_report->status_report.raddr.address);
+					sent_addr);
 
 	return TRUE;
 }
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 2/2] unit: national to international conversion
  2010-09-14 17:38 [RFC PATCH 1/2] smsutil: national to international conversion in status report Petteri Tikander
@ 2010-09-14 17:38 ` Petteri Tikander
  0 siblings, 0 replies; 6+ messages in thread
From: Petteri Tikander @ 2010-09-14 17:38 UTC (permalink / raw)
  To: ofono

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

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

diff --git a/unit/test-sms.c b/unit/test-sms.c
index 0ff9cd5..96b9b50 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -1316,6 +1316,23 @@ static void test_sr_assembly()
 
 	g_assert(id == 42);
 	g_assert(delivered == TRUE);
+	g_assert(g_hash_table_size(sra->assembly_table) == 0);
+
+	/*
+	 * Send sms-message in the national address-format,
+	 * but receive in the international address-format.
+	 */
+	sms_address_from_string(&addr, "9911630");
+	status_report_assembly_add_fragment(sra, 42, &addr, 4, time(NULL), 2);
+	status_report_assembly_add_fragment(sra, 42, &addr, 5, time(NULL), 2);
+
+	g_assert(!status_report_assembly_report(sra, &sr1, &id, &delivered));
+	g_assert(status_report_assembly_report(sra, &sr2, &id, &delivered));
+
+	g_assert(id == 42);
+	g_assert(delivered == TRUE);
+	g_assert(g_hash_table_size(sra->assembly_table) == 0);
+
 	status_report_assembly_free(sra);
 }
 
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/2] smsutil: national to international conversion in status report
  2010-09-17 17:24   ` Petteri Tikander
@ 2010-09-17 18:15     ` Denis Kenzior
  0 siblings, 0 replies; 6+ messages in thread
From: Denis Kenzior @ 2010-09-17 18:15 UTC (permalink / raw)
  To: ofono

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

Hi Petteri,

>> So my thinking is that for efficiency purposes we should try the direct
>> lookup first.  We don't want to pay the penalty of walking the entire
>> hash table every time for networks that are 'sane'.  If that fails, fall
>> back to the 'fuzzy' lookup.  This would also make the code a bit easier
>> to follow I think...
> 
> OK. Direct lookup would do this same and look nicer. But I think (but not 
> sure) that g_hash_table_lookup() does similar looping internally, as my 
> addition for while-looping address-tables. Well I was already taking 
> g_hash_table_lookup() back, but just making the code uglier when adding 
> 'international to national'-logic. That's because of iterating msg-id nodes, 
> so solution lead to two node-iteration loops.

Both looping and g_hash_table_lookup can be thought of as O(n) in the
worst case.  However, in the average case the hash table is way faster,
like O(1) or so.  The hash table is essentially an array, and the
hashing function computes an index in the array...

> 
> My idea was to find suitable address&Mr-pair. So if address matches but MR 
> doesn't, let's continue (while-loop inside while-loop).
> Was your idea actually that this inefficency-problem in looping occurs actually 
> in this point: if for some reason in sane networks the address matches, but MR 
> doesn't, function continues looping although it shouldn't? And then it loops 
> the entire table. In 'insane'-networks this is OK, because function is not 
> comparing necessarily the whole address?

So think of it like this, if a status report arrives from a 'sane
network', the g_hash_table_lookup (e.g. how we do it today) will give us
the sender address right away.  In the best case we compute the index of
the address hash table node in time O(1), more if there are collisions.
 We then perform a linear search on the status report assembly nodes
associated with that address.

If the address was found, but no MR was, then simply discard the status
report.  The fuzzy match will fail or give us the wrong answer anyway.

Only if the address was not found by direct lookup (e.g. the status
report is from an insane network or simply mis-addressed /
mis-delivered) should we go and try performing a fuzzy match.

In general fast-tracking the most common path is a good idea.  It makes
things seem faster in general.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/2] smsutil: national to international conversion in status report
  2010-09-15 16:10 ` Denis Kenzior
@ 2010-09-17 17:24   ` Petteri Tikander
  2010-09-17 18:15     ` Denis Kenzior
  0 siblings, 1 reply; 6+ messages in thread
From: Petteri Tikander @ 2010-09-17 17:24 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> Hi Petteri,
> 
> On 09/15/2010 02:25 AM, Petteri Tikander wrote:
> > ---
> >  src/smsutil.c |   70
> > ++++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed,
> > 55 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/smsutil.c b/src/smsutil.c
> > index 26c7951..2d47289 100644
> > --- a/src/smsutil.c
> > +++ b/src/smsutil.c
> > @@ -2826,14 +2826,18 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly, unsigned int offset =
> > status_report->status_report.mr / 32;
> >  	unsigned int bit = 1 << (status_report->status_report.mr % 32);
> >  	struct id_table_node *node = NULL;
> > -	const char *straddr;
> > +	const char *straddr, *sent_addr;
> > +	struct sms_address addr;
> >  	GHashTable *id_table;
> >  	gpointer key, value;
> >  	gboolean delivered;
> > -	GHashTableIter iter;
> > +	GHashTableIter iter_addr, iter;
> >  	gboolean pending;
> >  	int i;
> >  	unsigned int msg_id;
> > +	unsigned int n_digits;
> > +	unsigned int len_sent_addr;
> > +	unsigned int len_rec_addr;
> >
> >  	/* We ignore temporary or tempfinal status reports */
> >  	if (sr_st_to_delivered(status_report->status_report.st,
> > @@ -2841,20 +2845,54 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly, return FALSE;
> >
> >  	straddr = sms_address_to_string(&status_report->status_report.raddr);
> > -	id_table = g_hash_table_lookup(assembly->assembly_table, straddr);
> >
> > -	/* key (receiver address) does not exist in assembly */
> > -	if (id_table == NULL)
> > -		return FALSE;
> > +	g_hash_table_iter_init(&iter_addr, assembly->assembly_table);
> >
> > -	g_hash_table_iter_init(&iter, id_table);
> > -	while (g_hash_table_iter_next(&iter, &key, &value)) {
> > -		node = value;
> 
> So my thinking is that for efficiency purposes we should try the direct
> lookup first.  We don't want to pay the penalty of walking the entire
> hash table every time for networks that are 'sane'.  If that fails, fall
> back to the 'fuzzy' lookup.  This would also make the code a bit easier
> to follow I think...

OK. Direct lookup would do this same and look nicer. But I think (but not 
sure) that g_hash_table_lookup() does similar looping internally, as my 
addition for while-looping address-tables. Well I was already taking 
g_hash_table_lookup() back, but just making the code uglier when adding 
'international to national'-logic. That's because of iterating msg-id nodes, 
so solution lead to two node-iteration loops.

My idea was to find suitable address&Mr-pair. So if address matches but MR 
doesn't, let's continue (while-loop inside while-loop).
Was your idea actually that this inefficency-problem in looping occurs actually 
in this point: if for some reason in sane networks the address matches, but MR 
doesn't, function continues looping although it shouldn't? And then it loops 
the entire table. In 'insane'-networks this is OK, because function is not 
comparing necessarily the whole address?

> 
> > +	/*
> > +	 * Go through all addresses. Each address can relate to
> > +	 * 1-n msg_ids.
> > +	 */
> > +	while (g_hash_table_iter_next(&iter_addr, (gpointer) &sent_addr,
> > +					(gpointer) &id_table)) {
> > +		/*
> > +		 * Some networks can change address to international format,
> > +		 * although address is sent in the national format.
> > +		 * So notify this special case by checking only
> > +		 * last six digits. If address contains less than six digits,
> > +		 * compare only existing digits.
> > +		 */
> > +		if ((straddr[0] == '+') && (sent_addr[0] != '+')) {
> 
> In theory it could also be vice-versa...

OK. Let's also taking care of 'international to national'-case.

> 
> > +			len_sent_addr = strlen(sent_addr);
> > +			len_rec_addr = strlen(straddr);
> > +			n_digits = (len_sent_addr > 6) ? 6 : len_sent_addr;
> >
> > -		if (node->mrs[offset] & bit)
> > -			break;
> > +			if (strcmp(&straddr[len_rec_addr - n_digits],
> > +					&sent_addr[len_sent_addr - n_digits]))
> 
> Perhaps we should simply start at the last position of both strings and
> compare in reverse.  If the match is MIN(6, strlen(sent_addr))
> characters, then accept this as a 'fuzzy' match.
> 

OK. Lets' compare single characters in the reverse order.

> > +				continue;
> > +		}
> > +		/*
> > +		 * In other cases the whole number received in the status report
> > +		 * should match with the number originally sent.
> > +		 */
> > +		else if (strcmp(straddr, sent_addr))
> > +				continue;
> > +
> > +		g_hash_table_iter_init(&iter, id_table);
> > +		while (g_hash_table_iter_next(&iter, &key, &value)) {
> > +			node = value;
> > +
> > +			if (node->mrs[offset] & bit)
> > +				break;
> >
> > -		node = NULL;
> > +			node = NULL;
> > +		}
> > +
> > +		/*
> > +		 * Received address with MR matched with one of the stored
> > +		 * addresses and MR, so no need to continue searching.
> > +		 */
> > +		if (node)
> > +			break;
> >  	}
> >
> >  	/* Unable to find a message reference belonging to this address */
> > @@ -2881,6 +2919,8 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly,
> >
> >  	msg_id = *(unsigned int *) key;
> >
> > +	sms_address_from_string(&addr, sent_addr);
> > +
> >  	if (pending == TRUE && node->deliverable == TRUE) {
> >  		/*
> >  		 * More status reports expected, and already received
> > @@ -2888,7 +2928,7 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly, */
> >  		sr_assembly_add_fragment_backup(
> >  					assembly->imsi, node,
> > -					&status_report->status_report.raddr,
> > +					&addr,
> >  					msg_id);
> >
> >  		return FALSE;
> > @@ -2901,14 +2941,14 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly, *out_id = msg_id;
> >
> >  	sr_assembly_remove_fragment_backup(assembly->imsi,
> > -					&status_report->status_report.raddr,
> > +					&addr,
> >  					msg_id);
> >
> >  	g_hash_table_iter_remove(&iter);
> >
> >  	if (g_hash_table_size(id_table) == 0)
> >  		g_hash_table_remove(assembly->assembly_table,
> > -				status_report->status_report.raddr.address);
> > +					sent_addr);
> >
> >  	return TRUE;
> >  }
> 
> Otherwise it looks good.
> 
> Regards,
> -Denis
> 

Have a nice week end.

Br, Petteri


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/2] smsutil: national to international conversion in status report
  2010-09-15  7:25 [RFC PATCH 1/2] smsutil: national to international conversion in status report Petteri Tikander
@ 2010-09-15 16:10 ` Denis Kenzior
  2010-09-17 17:24   ` Petteri Tikander
  0 siblings, 1 reply; 6+ messages in thread
From: Denis Kenzior @ 2010-09-15 16:10 UTC (permalink / raw)
  To: ofono

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

Hi Petteri,

On 09/15/2010 02:25 AM, Petteri Tikander wrote:
> ---
>  src/smsutil.c |   70 ++++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 26c7951..2d47289 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -2826,14 +2826,18 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
>  	unsigned int offset = status_report->status_report.mr / 32;
>  	unsigned int bit = 1 << (status_report->status_report.mr % 32);
>  	struct id_table_node *node = NULL;
> -	const char *straddr;
> +	const char *straddr, *sent_addr;
> +	struct sms_address addr;
>  	GHashTable *id_table;
>  	gpointer key, value;
>  	gboolean delivered;
> -	GHashTableIter iter;
> +	GHashTableIter iter_addr, iter;
>  	gboolean pending;
>  	int i;
>  	unsigned int msg_id;
> +	unsigned int n_digits;
> +	unsigned int len_sent_addr;
> +	unsigned int len_rec_addr;
>  
>  	/* We ignore temporary or tempfinal status reports */
>  	if (sr_st_to_delivered(status_report->status_report.st,
> @@ -2841,20 +2845,54 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
>  		return FALSE;
>  
>  	straddr = sms_address_to_string(&status_report->status_report.raddr);
> -	id_table = g_hash_table_lookup(assembly->assembly_table, straddr);
>  
> -	/* key (receiver address) does not exist in assembly */
> -	if (id_table == NULL)
> -		return FALSE;
> +	g_hash_table_iter_init(&iter_addr, assembly->assembly_table);
>  
> -	g_hash_table_iter_init(&iter, id_table);
> -	while (g_hash_table_iter_next(&iter, &key, &value)) {
> -		node = value;

So my thinking is that for efficiency purposes we should try the direct
lookup first.  We don't want to pay the penalty of walking the entire
hash table every time for networks that are 'sane'.  If that fails, fall
back to the 'fuzzy' lookup.  This would also make the code a bit easier
to follow I think...

> +	/*
> +	 * Go through all addresses. Each address can relate to
> +	 * 1-n msg_ids.
> +	 */
> +	while (g_hash_table_iter_next(&iter_addr, (gpointer) &sent_addr,
> +					(gpointer) &id_table)) {
> +		/*
> +		 * Some networks can change address to international format,
> +		 * although address is sent in the national format.
> +		 * So notify this special case by checking only
> +		 * last six digits. If address contains less than six digits,
> +		 * compare only existing digits.
> +		 */
> +		if ((straddr[0] == '+') && (sent_addr[0] != '+')) {

In theory it could also be vice-versa...

> +			len_sent_addr = strlen(sent_addr);
> +			len_rec_addr = strlen(straddr);
> +			n_digits = (len_sent_addr > 6) ? 6 : len_sent_addr;
>  
> -		if (node->mrs[offset] & bit)
> -			break;
> +			if (strcmp(&straddr[len_rec_addr - n_digits],
> +					&sent_addr[len_sent_addr - n_digits]))

Perhaps we should simply start at the last position of both strings and
compare in reverse.  If the match is MIN(6, strlen(sent_addr))
characters, then accept this as a 'fuzzy' match.

> +				continue;
> +		}
> +		/*
> +		 * In other cases the whole number received in the status report
> +		 * should match with the number originally sent.
> +		 */
> +		else if (strcmp(straddr, sent_addr))
> +				continue;
> +
> +		g_hash_table_iter_init(&iter, id_table);
> +		while (g_hash_table_iter_next(&iter, &key, &value)) {
> +			node = value;
> +
> +			if (node->mrs[offset] & bit)
> +				break;
>  
> -		node = NULL;
> +			node = NULL;
> +		}
> +
> +		/*
> +		 * Received address with MR matched with one of the stored
> +		 * addresses and MR, so no need to continue searching.
> +		 */
> +		if (node)
> +			break;
>  	}
>  
>  	/* Unable to find a message reference belonging to this address */
> @@ -2881,6 +2919,8 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
>  
>  	msg_id = *(unsigned int *) key;
>  
> +	sms_address_from_string(&addr, sent_addr);
> +
>  	if (pending == TRUE && node->deliverable == TRUE) {
>  		/*
>  		 * More status reports expected, and already received
> @@ -2888,7 +2928,7 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
>  		 */
>  		sr_assembly_add_fragment_backup(
>  					assembly->imsi, node,
> -					&status_report->status_report.raddr,
> +					&addr,
>  					msg_id);
>  
>  		return FALSE;
> @@ -2901,14 +2941,14 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
>  		*out_id = msg_id;
>  
>  	sr_assembly_remove_fragment_backup(assembly->imsi,
> -					&status_report->status_report.raddr,
> +					&addr,
>  					msg_id);
>  
>  	g_hash_table_iter_remove(&iter);
>  
>  	if (g_hash_table_size(id_table) == 0)
>  		g_hash_table_remove(assembly->assembly_table,
> -				status_report->status_report.raddr.address);
> +					sent_addr);
>  
>  	return TRUE;
>  }

Otherwise it looks good.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/2] smsutil: national to international conversion in status report
@ 2010-09-15  7:25 Petteri Tikander
  2010-09-15 16:10 ` Denis Kenzior
  0 siblings, 1 reply; 6+ messages in thread
From: Petteri Tikander @ 2010-09-15  7:25 UTC (permalink / raw)
  To: ofono

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

---
 src/smsutil.c |   70 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index 26c7951..2d47289 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2826,14 +2826,18 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 	unsigned int offset = status_report->status_report.mr / 32;
 	unsigned int bit = 1 << (status_report->status_report.mr % 32);
 	struct id_table_node *node = NULL;
-	const char *straddr;
+	const char *straddr, *sent_addr;
+	struct sms_address addr;
 	GHashTable *id_table;
 	gpointer key, value;
 	gboolean delivered;
-	GHashTableIter iter;
+	GHashTableIter iter_addr, iter;
 	gboolean pending;
 	int i;
 	unsigned int msg_id;
+	unsigned int n_digits;
+	unsigned int len_sent_addr;
+	unsigned int len_rec_addr;
 
 	/* We ignore temporary or tempfinal status reports */
 	if (sr_st_to_delivered(status_report->status_report.st,
@@ -2841,20 +2845,54 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		return FALSE;
 
 	straddr = sms_address_to_string(&status_report->status_report.raddr);
-	id_table = g_hash_table_lookup(assembly->assembly_table, straddr);
 
-	/* key (receiver address) does not exist in assembly */
-	if (id_table == NULL)
-		return FALSE;
+	g_hash_table_iter_init(&iter_addr, assembly->assembly_table);
 
-	g_hash_table_iter_init(&iter, id_table);
-	while (g_hash_table_iter_next(&iter, &key, &value)) {
-		node = value;
+	/*
+	 * Go through all addresses. Each address can relate to
+	 * 1-n msg_ids.
+	 */
+	while (g_hash_table_iter_next(&iter_addr, (gpointer) &sent_addr,
+					(gpointer) &id_table)) {
+		/*
+		 * Some networks can change address to international format,
+		 * although address is sent in the national format.
+		 * So notify this special case by checking only
+		 * last six digits. If address contains less than six digits,
+		 * compare only existing digits.
+		 */
+		if ((straddr[0] == '+') && (sent_addr[0] != '+')) {
+			len_sent_addr = strlen(sent_addr);
+			len_rec_addr = strlen(straddr);
+			n_digits = (len_sent_addr > 6) ? 6 : len_sent_addr;
 
-		if (node->mrs[offset] & bit)
-			break;
+			if (strcmp(&straddr[len_rec_addr - n_digits],
+					&sent_addr[len_sent_addr - n_digits]))
+				continue;
+		}
+		/*
+		 * In other cases the whole number received in the status report
+		 * should match with the number originally sent.
+		 */
+		else if (strcmp(straddr, sent_addr))
+				continue;
+
+		g_hash_table_iter_init(&iter, id_table);
+		while (g_hash_table_iter_next(&iter, &key, &value)) {
+			node = value;
+
+			if (node->mrs[offset] & bit)
+				break;
 
-		node = NULL;
+			node = NULL;
+		}
+
+		/*
+		 * Received address with MR matched with one of the stored
+		 * addresses and MR, so no need to continue searching.
+		 */
+		if (node)
+			break;
 	}
 
 	/* Unable to find a message reference belonging to this address */
@@ -2881,6 +2919,8 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 
 	msg_id = *(unsigned int *) key;
 
+	sms_address_from_string(&addr, sent_addr);
+
 	if (pending == TRUE && node->deliverable == TRUE) {
 		/*
 		 * More status reports expected, and already received
@@ -2888,7 +2928,7 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		 */
 		sr_assembly_add_fragment_backup(
 					assembly->imsi, node,
-					&status_report->status_report.raddr,
+					&addr,
 					msg_id);
 
 		return FALSE;
@@ -2901,14 +2941,14 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		*out_id = msg_id;
 
 	sr_assembly_remove_fragment_backup(assembly->imsi,
-					&status_report->status_report.raddr,
+					&addr,
 					msg_id);
 
 	g_hash_table_iter_remove(&iter);
 
 	if (g_hash_table_size(id_table) == 0)
 		g_hash_table_remove(assembly->assembly_table,
-				status_report->status_report.raddr.address);
+					sent_addr);
 
 	return TRUE;
 }
-- 
1.6.3.3



^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-09-17 18:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-14 17:38 [RFC PATCH 1/2] smsutil: national to international conversion in status report Petteri Tikander
2010-09-14 17:38 ` [RFC PATCH 2/2] unit: national to international conversion Petteri Tikander
2010-09-15  7:25 [RFC PATCH 1/2] smsutil: national to international conversion in status report Petteri Tikander
2010-09-15 16:10 ` Denis Kenzior
2010-09-17 17:24   ` Petteri Tikander
2010-09-17 18:15     ` Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.