On 31.03.2011 13:04, Holger Hans Peter Freyther wrote:
Hi Dennis,sorry for the delay and thanks a lot for your patch. Do you feel like adding a test case to the libosmocore/tests/sms/sms_test.c? thanks a lot holger
Hi Holger No Problem. See attach for the new sms_lib.patch Best regards Dennis
>From 9a35914bc1c7b78d13ec400350eb8639e8125a0e Mon Sep 17 00:00:00 2001 From: Dennis Wehrle <[email protected]> Date: Tue, 22 Mar 2011 18:25:53 +0100 Subject: [PATCH 1/2] SMS-fix: due to a misuse of user_data_len the sms where cropped (on vty) and wrongly stored on the database. Therefore two new functions was created: uint8_t gsm_get_octet_len(const uint8_t sept_len) and uint8_t gsm_get_septet_len(const char *text) --- include/osmocore/gsm_utils.h | 3 ++ src/gsm_utils.c | 54 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/include/osmocore/gsm_utils.h b/include/osmocore/gsm_utils.h index 0aadd2e..a47ce7b 100644 --- a/include/osmocore/gsm_utils.h +++ b/include/osmocore/gsm_utils.h @@ -59,6 +59,9 @@ enum gsm_band gsm_band_parse(const char *mhz); int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length); int gsm_7bit_encode(uint8_t *result, const char *data); +uint8_t gsm_get_octet_len(const uint8_t sept_len); +uint8_t gsm_get_septet_len(const char *text); + int ms_pwr_ctl_lvl(enum gsm_band band, unsigned int dbm); int ms_pwr_dbm(enum gsm_band band, uint8_t lvl); diff --git a/src/gsm_utils.c b/src/gsm_utils.c index 31e3cd6..faac7cf 100644 --- a/src/gsm_utils.c +++ b/src/gsm_utils.c @@ -73,17 +73,52 @@ static int gsm_septet_lookup(uint8_t ch) return -1; } +/* Warning, user_data_len indicates the amount of septets (characters), we need amount of octets occupied */ +/* compute the number of octets: */ +/* #octets = (#septets * 7 bit) / 8 bit */ +/* e.g. 47 non-extension characters needs 329 bits / 8 bit = 41,125 octets => we need 42 octets for 47 characters */ +uint8_t gsm_get_octet_len(const uint8_t sept_len){ + int octet_len = (sept_len * 7) / 8; + if ((sept_len * 7) % 8 != 0) + octet_len++; + + return octet_len; +} + + +/* Get number of septets. Each extension character has to be count twice, because they need 2 septets*/ +uint8_t gsm_get_septet_len(const char *text){ + uint8_t septet_len = strlen(text); + int i; + for(i = 0; i < strlen(text); i++){ + switch(text[i]){ + case 0x0c: + case 0x5e: + case 0x7b: + case 0x7d: + case 0x5c: + case 0x5b: + case 0x7e: + case 0x5d: + case 0x7c: + septet_len++; + default: + continue; + } + } + return septet_len; +} + /* GSM 03.38 6.2.1 Character unpacking */ int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t length) { int i = 0; - int l = 0; - int septet_l = (length * 8) / 7; + + int septet_l = length; uint8_t *rtext = calloc(septet_l, sizeof(uint8_t)); uint8_t tmp; /* FIXME: We need to account for user data headers here */ - i += l; for (; i < septet_l; i++){ rtext[i] = ((user_data[(i * 7 + 7) >> 3] << @@ -155,6 +190,12 @@ int gsm_7bit_encode(uint8_t *result, const char *data) shift = 0; continue; } + /* special end case. This is necessary if the last septet fits into the previous octet. */ + /* E.g. 48 non-extension characters: ....ag ( a = 1100001, g = 1100111) */ + /* result[40] = 100001 XX */ + /* result[41] = 1100111 1 */ + if(shift == 7 && i + 1 == y) + break; cb = (rdata[i] & 0x7f) >> shift; if(i + 1 < y){ @@ -168,7 +209,12 @@ int gsm_7bit_encode(uint8_t *result, const char *data) } free(rdata); - return z; + + /* we don't care about the number of octets, because they are not unique. E.g.: */ + /* 1.) 46 non-extension characters + 1 extension character => (46 * 7 bit + (1 * (2 * 7 bit))) / 8 bit = 42 octets */ + /* 2.) 47 non-extension characters => (47 * 7 bit) / 8 bit = 41,125 = 42 octets */ + /* 3.) 48 non-extension characters => (48 * 7 bit) / 8 bit = 42 octects */ + return y; } /* determine power control level for given dBm value, as indicated -- 1.7.1 >From 26b7a3aa75eead0e81e66928f4169d330f165c28 Mon Sep 17 00:00:00 2001 From: Dennis Wehrle <[email protected]> Date: Fri, 1 Apr 2011 14:13:44 +0200 Subject: [PATCH 2/2] Add: testcases for the sms bugfix. --- tests/sms/sms_test.c | 96 ++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 82 insertions(+), 14 deletions(-) diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index 9d87b5b..ff11e2e 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -32,15 +32,18 @@ struct test_case { const uint16_t input_length; const uint8_t *expected; - const uint16_t expected_length; + const uint16_t expected_octet_length; + const uint16_t expected_septet_length; }; static const char simple_text[] = "test text"; +#define simple_septet_length 9 static const uint8_t simple_enc[] = { 0xf4, 0xf2, 0x9c, 0x0e, 0xa2, 0x97, 0xf1, 0x74 }; -static const char escape_text[] = "!$ a more#^- complicated test@@?_\%! case"; +static const char escape_text[] = "!$ a more#^- complicated test@@?_%! case"; +#define escape_septet_length 41 /* note: the ^ counts as two, because it is a extension character */ static const uint8_t escape_enc[] = { 0x21, 0x01, 0x28, 0x0c, 0x6a, 0xbf, 0xe5, 0xe5, 0xd1, 0x86, 0xd2, 0x02, 0x8d, 0xdf, 0x6d, 0x38, 0x3b, 0x3d, @@ -48,17 +51,49 @@ static const uint8_t escape_enc[] = { 0x00, 0xbf, 0x48, 0x29, 0x04, 0x1a, 0x87, 0xe7, 0x65, }; +static const char enhanced_text[] = "enhanced ^ {][} test |+~ ^ test"; +#define enhanced_septet_length 39 /* note: the characters { } [ ] ^ | ~ count as two (each of them), because they are extension characters */ +static const uint8_t enhanced_enc[] = { + 0x65, 0x37, 0x3A, 0xEC, 0x1E, 0x97, 0xC9, 0xA0, 0x0D, + 0x05, 0xB4, 0x41, 0x6D, 0x7C, 0x1B, 0xDE, 0x26, 0x05, + 0xA2, 0x97, 0xE7, 0x74, 0xD0, 0x06, 0xB8, 0xDA, 0xF4, + 0x40, 0x1B, 0x0A, 0x88, 0x5E, 0x9E, 0xD3, 0x01, +}; + +static const char enhancedV2_text[] = "enhanced ^ {][} test |+~ ^ tests"; +#define enhancedV2_septet_length 40 /* note: number of octets are equal to the enhanced_text! */ +static const uint8_t enhancedV2_enc[] = { + 0x65, 0x37, 0x3A, 0xEC, 0x1E, 0x97, 0xC9, 0xA0, 0x0D, + 0x05, 0xB4, 0x41, 0x6D, 0x7C, 0x1B, 0xDE, 0x26, 0x05, + 0xA2, 0x97, 0xE7, 0x74, 0xD0, 0x06, 0xB8, 0xDA, 0xF4, + 0x40, 0x1B, 0x0A, 0x88, 0x5E, 0x9E, 0xD3, 0xE7, +}; + static const struct test_case test_encode[] = { { .input = simple_text, .expected = simple_enc, - .expected_length = sizeof(simple_enc), + .expected_octet_length = sizeof(simple_enc), + .expected_septet_length = simple_septet_length, }, { .input = escape_text, .expected = escape_enc, - .expected_length = sizeof(escape_enc), + .expected_octet_length = sizeof(escape_enc), + .expected_septet_length = escape_septet_length, + }, + { + .input = enhanced_text, + .expected = enhanced_enc, + .expected_octet_length = sizeof(enhanced_enc), + .expected_septet_length = enhanced_septet_length, + }, + { + .input = enhancedV2_text, + .expected = enhancedV2_enc, + .expected_octet_length = sizeof(enhancedV2_enc), + .expected_septet_length = enhancedV2_septet_length, }, }; @@ -68,11 +103,25 @@ static const struct test_case test_decode[] = .input = simple_enc, .input_length = sizeof(simple_enc), .expected = simple_text, + .expected_septet_length = simple_septet_length, }, { .input = escape_enc, .input_length = sizeof(escape_enc), .expected = escape_text, + .expected_septet_length = escape_septet_length, + }, + { + .input = enhanced_enc, + .input_length = sizeof(enhanced_enc), + .expected = enhanced_text, + .expected_septet_length = enhanced_septet_length, + }, + { + .input = enhancedV2_enc, + .input_length = sizeof(enhancedV2_enc), + .expected = enhancedV2_text, + .expected_septet_length = enhancedV2_septet_length, }, }; @@ -83,23 +132,38 @@ int main(int argc, char** argv) uint8_t *sms; uint8_t i; - uint8_t length; + uint8_t octet_length; + uint8_t septet_length; + uint8_t gsm_septet_length; uint8_t coded[256]; char result[256]; /* test 7-bit encoding */ for (i = 0; i < ARRAY_SIZE(test_encode); ++i) { memset(coded, 0x42, sizeof(coded)); - length = gsm_7bit_encode(coded, test_encode[i].input); + septet_length = gsm_7bit_encode(coded, test_encode[i].input); + octet_length = gsm_get_octet_len(septet_length); + gsm_septet_length = gsm_get_septet_len(test_encode[i].input); + if (octet_length != test_encode[i].expected_octet_length) { + fprintf(stderr, "Encode case %d: Octet length failure. Got %d, expected %d\n", + i, octet_length, test_encode[i].expected_octet_length); + return -1; + } + + if (septet_length != test_encode[i].expected_septet_length){ + fprintf(stderr, "Encode case %d: Septet length failure. Got %d, expected %d\n", + i, septet_length, test_encode[i].expected_septet_length); + return -1; + } - if (length != test_encode[i].expected_length) { - fprintf(stderr, "Failed to encode case %d. Got %d, expected %d\n", - i, length, test_encode[i].expected_length); + if (gsm_septet_length != test_encode[i].expected_septet_length){ + fprintf(stderr, "Encode case %d: GSM-Septet length failure. Got %d, expected %d\n", + i, septet_length, test_encode[i].expected_septet_length); return -1; } - if (memcmp(coded, test_encode[i].expected, length) != 0) { - fprintf(stderr, "Encoded content does not match for %d\n", + if (memcmp(coded, test_encode[i].expected, octet_length) != 0) { + fprintf(stderr, "Encoded content does not match for case %d\n", i); return -1; } @@ -108,13 +172,17 @@ int main(int argc, char** argv) /* test 7-bit decoding */ for (i = 0; i < ARRAY_SIZE(test_decode); ++i) { memset(result, 0x42, sizeof(coded)); - gsm_7bit_decode(result, test_decode[i].input, - test_decode[i].input_length); - + septet_length = gsm_7bit_decode(result, test_decode[i].input, + test_decode[i].expected_septet_length); if (strcmp(result, test_decode[i].expected) != 0) { fprintf(stderr, "Test case %d failed to decode.\n", i); return -1; } + if (septet_length != test_decode[i].expected_septet_length){ + fprintf(stderr, "Decode case %d: Septet length failure. Got %d, expected %d\n", + i, septet_length, test_decode[i].expected_septet_length); + return -1; + } } printf("OK\n"); -- 1.7.1
