Hi,
* Holger Hans Peter Freyther <hol...@freyther.de> [2010-07-19 22:13]:
> On 07/10/2010 12:56 AM, Nico Golde wrote:
> > Alright, cleaned up the previous code a bit, implemented the
> > decoder and added a test case which the previous code
> > encoded incorrect. Please note that just using the new test
> > case string in the old code will also pass the test even
> > though the encoding is wrong. You need to compare the
> > encoded values to e.g. output of pduspy[0]. Patch attached.
> 
> I have finally applied your patch, thanks for poking me. I have
> also made two modifications. The first is to separate testing of
> encoding/decoding and comparing it to a byte array/string, the
> second is to fix a possible off by one.

Good catch, I already had this fixed locally but was waiting 
with another patch until you have looked at this one. Will 
repost an update in case something like this occurs again to 
prevent this doubled work.

> I used the current result of the encoding and there seems to
> be at least one more bug with our encoding routines. We append
> zero byte(s) at the end that are not touched by the encoding
> routine but covered by the length.
> 
> A nice way to test this is to change the memset(something, 0 to
> memset(something, 23 and see how it is going to change.
> 
> Would you be interested in changing that?

I just looked into this and if I'm not mistaken this is no 
bug in the encoding but just a bug when comparing the 
encoded result. The current code memcmp's n bytes where n is 
the result of gsm_7bit_encode() with the input string 
bytewise. The problem with this is that the function returns 
the number of septets not octets. Attached is a patch for 
the test code that should fix this.

Cheers
Nico
>From d9765553adc8308d7f1aaa33dc30aa48a4a17d63 Mon Sep 17 00:00:00 2001
From: Nico Golde <n...@ngolde.de>
Date: Mon, 19 Jul 2010 23:01:50 +0200
Subject: [PATCH] * when comparing the coded result with the expected coded values, do
   not compare n bytes where n is the result of gsm_7bit_encode() but
   the actual length of the input string as gsm_7bit_encode() returns
   the number of septets not octets

---
 tests/sms/sms_test.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c
index 3742dd8..cab91ab 100644
--- a/tests/sms/sms_test.c
+++ b/tests/sms/sms_test.c
@@ -99,7 +99,7 @@ int main(int argc, char** argv)
 			return -1;
 		}
 
-		if (memcmp(coded, test_encode[i].expected, length) != 0) {
+		if (memcmp(coded, test_encode[i].expected, strlen(test_encode[i].expected)) != 0) {
 			fprintf(stderr, "Encoded content does not match for %d\n",
 				i);
 			return -1;
-- 
1.7.1

Reply via email to