From: Max <[email protected]>
Refactor and simplify gsm 7 bit decoder.
Remove useless tests for legacy functions.
Make tests more human-readable.
Replace assert inside the library with returning error code.
Ticket: OW#1198
Sponsored-by: On-Waves ehf
---
src/gsm/gsm_utils.c | 35 +++++++++++++++++-------------
tests/sms/sms_test.c | 59 ++++++++++++++++++++++++---------------------------
tests/sms/sms_test.ok | 28 +++++++++---------------
3 files changed, 58 insertions(+), 64 deletions(-)
diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c
index fad59bc..bcc0271 100644
--- a/src/gsm/gsm_utils.c
+++ b/src/gsm/gsm_utils.c
@@ -126,18 +126,17 @@ uint8_t gsm_get_octet_len(const uint8_t sept_len){
return octet_len;
}
-/* GSM 03.38 6.2.1 Character unpacking */
+/* GSM 03.38 6.2.1 Character unpacking
+ * returns -1 on size errors, number of characters otherwise
+ */
int gsm_7bit_decode_n_hdr(char *text, size_t n, const uint8_t *user_data,
uint8_t septet_l, uint8_t ud_hdr_ind)
{
- int i = 0;
- int shift = 0;
- uint8_t c7, c8;
- uint8_t next_is_ext = 0;
+ unsigned shift = 0;
+ uint8_t c7, c8, next_is_ext = 0, lu, ru, maxlen =
gsm_get_octet_len(septet_l);
const char *text_buf_begin = text;
const char *text_buf_end = text + n;
- int nchars;
- OSMO_ASSERT (n > 0);
+ if (n < 1) return -1;
/* skip the user data header */
if (ud_hdr_ind) {
@@ -148,12 +147,20 @@ int gsm_7bit_decode_n_hdr(char *text, size_t n, const
uint8_t *user_data, uint8_
septet_l = septet_l - shift;
}
+ unsigned i, l, r;
for (i = 0; i < septet_l && text != text_buf_end - 1; i++) {
- c7 =
- ((user_data[((i + shift) * 7 + 7) >> 3] <<
- (7 - (((i + shift) * 7 + 7) & 7))) |
- (user_data[((i + shift) * 7) >> 3] >>
- (((i + shift) * 7) & 7))) & 0x7f;
+
+ l = ((i + shift) * 7 + 7) >> 3;
+ r = ((i + shift) * 7) >> 3;
+
+ if (l >= maxlen)
+ lu = 0; // workaround to prevent getting out of bounds
+ else
+ lu = user_data[l] << (7 - (((i + shift) * 7 + 7) & 7));
+
+ ru = user_data[r] >> (((i + shift) * 7) & 7);
+
+ c7 = (lu | ru) & 0x7f;
if (next_is_ext) {
/* this is an extension character */
@@ -169,11 +176,9 @@ int gsm_7bit_decode_n_hdr(char *text, size_t n, const
uint8_t *user_data, uint8_
*(text++) = c8;
}
- nchars = text - text_buf_begin;
-
*text = '\0';
- return nchars;
+ return text - text_buf_begin;
}
int gsm_7bit_decode_n(char *text, size_t n, const uint8_t *user_data, uint8_t
septet_l)
diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c
index cdd4158..1927925 100644
--- a/tests/sms/sms_test.c
+++ b/tests/sms/sms_test.c
@@ -44,6 +44,7 @@ struct test_case {
const uint16_t expected_octet_length;
const uint16_t expected_septet_length;
const uint8_t ud_hdr_ind;
+ const char * descr;
};
static const char simple_text[] = "test text";
@@ -131,6 +132,7 @@ static const struct test_case test_multiple_encode[] =
.expected_octet_length = sizeof(concatenated_part1_enc),
.expected_septet_length = concatenated_part1_septet_length,
.ud_hdr_ind = 1,
+ .descr = "concatenated text p1"
},
{
.input = (const uint8_t *) concatenated_text,
@@ -138,6 +140,7 @@ static const struct test_case test_multiple_encode[] =
.expected_octet_length = sizeof(concatenated_part2_enc),
.expected_septet_length = concatenated_part2_septet_length,
.ud_hdr_ind = 1,
+ .descr = "concatenated text p2"
},
};
@@ -149,6 +152,7 @@ static const struct test_case test_encode[] =
.expected_octet_length = sizeof(simple_enc),
.expected_septet_length = simple_septet_length,
.ud_hdr_ind = 0,
+ .descr = "simple text"
},
{
.input = (const uint8_t *) escape_text,
@@ -156,6 +160,7 @@ static const struct test_case test_encode[] =
.expected_octet_length = sizeof(escape_enc),
.expected_septet_length = escape_septet_length,
.ud_hdr_ind = 0,
+ .descr = "escape text"
},
{
.input = (const uint8_t *) enhanced_text,
@@ -163,6 +168,7 @@ static const struct test_case test_encode[] =
.expected_octet_length = sizeof(enhanced_enc),
.expected_septet_length = enhanced_septet_length,
.ud_hdr_ind = 0,
+ .descr = "enhanced text"
},
{
.input = (const uint8_t *) enhancedV2_text,
@@ -170,6 +176,7 @@ static const struct test_case test_encode[] =
.expected_octet_length = sizeof(enhancedV2_enc),
.expected_septet_length = enhancedV2_septet_length,
.ud_hdr_ind = 0,
+ .descr = "enhanced text v2"
},
};
@@ -181,6 +188,7 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) simple_text,
.expected_septet_length = simple_septet_length,
.ud_hdr_ind = 0,
+ .descr = "simple text"
},
{
.input = escape_enc,
@@ -188,6 +196,7 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) escape_text,
.expected_septet_length = escape_septet_length,
.ud_hdr_ind = 0,
+ .descr = "escape text"
},
{
.input = enhanced_enc,
@@ -195,6 +204,7 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) enhanced_text,
.expected_septet_length = enhanced_septet_length,
.ud_hdr_ind = 0,
+ .descr = "enhanced text"
},
{
.input = enhancedV2_enc,
@@ -202,6 +212,7 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) enhancedV2_text,
.expected_septet_length = enhancedV2_septet_length,
.ud_hdr_ind = 0,
+ .descr = "enhanced text v2"
},
{
.input = concatenated_part1_enc,
@@ -209,6 +220,7 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) splitted_text_part1,
.expected_septet_length =
concatenated_part1_septet_length_with_header,
.ud_hdr_ind = 1,
+ .descr = "concatenated text p1"
},
{
.input = concatenated_part2_enc,
@@ -216,7 +228,8 @@ static const struct test_case test_decode[] =
.expected = (const uint8_t *) splitted_text_part2,
.expected_septet_length =
concatenated_part2_septet_length_with_header,
.ud_hdr_ind = 1,
- },
+ .descr = "concatenated text p2"
+ },
};
static void test_octet_return()
@@ -288,29 +301,17 @@ int main(int argc, char** argv)
/* test 7-bit encoding */
for (i = 0; i < ARRAY_SIZE(test_encode); ++i) {
- /* Test legacy function (return value only) */
- septet_length = gsm_7bit_encode(coded,
- (const char *)
test_encode[i].input);
- printf("Legacy encode case %d: "
- "septet length %d (expected %d)\n"
- , i
- , septet_length, test_encode[i].expected_septet_length
- );
- OSMO_ASSERT (septet_length ==
test_encode[i].expected_septet_length);
-
- /* Test new function */
memset(coded, 0x42, sizeof(coded));
septet_length = gsm_7bit_encode_n(coded, sizeof(coded),
(const char *)
test_encode[i].input,
&octets_written);
computed_octet_length = gsm_get_octet_len(septet_length);
- printf("Encode case %d: "
- "Octet length %d (expected %d, computed %d), "
- "septet length %d (expected %d)\n"
- , i
- , octets_written, test_encode[i].expected_octet_length,
computed_octet_length
- , septet_length, test_encode[i].expected_septet_length
- );
+ printf("Encode case %d (%s): "
+ "Octet length %d (expected %d, computed %d, test %s), "
+ "septet length %d (expected %d, test %s)\n",
+ i, test_encode[i].descr,
+ octets_written, test_encode[i].expected_octet_length,
computed_octet_length, (test_encode[i].expected_octet_length ==
computed_octet_length) ? "OK" : "FAIL",
+ septet_length, test_encode[i].expected_septet_length,
(septet_length == test_encode[i].expected_septet_length) ? "OK" : "FAIL");
OSMO_ASSERT (octets_written ==
test_encode[i].expected_octet_length);
OSMO_ASSERT (octets_written == computed_octet_length);
@@ -377,22 +378,18 @@ int main(int argc, char** argv)
/* test 7-bit decoding */
for (i = 0; i < ARRAY_SIZE(test_decode); ++i) {
- /* Test legacy function (return value only) */
- if (!test_decode[i].ud_hdr_ind) {
- nchars = gsm_7bit_decode(result, test_decode[i].input,
-
test_decode[i].expected_septet_length);
- printf("Legacy decode case %d: "
- "return value %d (expected %d)\n",
- i, nchars,
test_decode[i].expected_septet_length);
- }
-
- /* Test new function */
memset(result, 0x42, sizeof(result));
+// printf("Attempting to 7 bit decode %d to %d with exp %d and ind
%d\n", test_decode[i].input_length, sizeof(result),
test_decode[i].expected_septet_length, test_decode[i].ud_hdr_ind);
nchars = gsm_7bit_decode_n_hdr(result, sizeof(result),
test_decode[i].input,
test_decode[i].expected_septet_length,
test_decode[i].ud_hdr_ind);
- printf("Decode case %d: return value %d (expected %d)\n", i,
nchars, strlen(result));
+ printf("Decode case %d (%s): return value %d, expected %d, test
%s\n", i, test_decode[i].descr, nchars, strlen(result), (strlen(result) ==
nchars) ? "OK" : "FAIL");
+
+ int res = strcmp(result, (const char *)
test_decode[i].expected);
+ if(0 != res) {
+ printf("%d:\n%s\n%s\n", res, osmo_hexdump(result,
strlen(result)),
+ osmo_hexdump(test_decode[i].expected, strlen(result)));
+ }
- OSMO_ASSERT(strcmp(result, (const char *)
test_decode[i].expected) == 0);
OSMO_ASSERT(nchars == strlen(result));
/* check buffer limiting */
diff --git a/tests/sms/sms_test.ok b/tests/sms/sms_test.ok
index fa536ea..144e602 100644
--- a/tests/sms/sms_test.ok
+++ b/tests/sms/sms_test.ok
@@ -1,22 +1,14 @@
SMS testing
-Legacy encode case 0: septet length 9 (expected 9)
-Encode case 0: Octet length 8 (expected 8, computed 8), septet length 9
(expected 9)
-Legacy encode case 1: septet length 41 (expected 41)
-Encode case 1: Octet length 36 (expected 36, computed 36), septet length 41
(expected 41)
-Legacy encode case 2: septet length 39 (expected 39)
-Encode case 2: Octet length 35 (expected 35, computed 35), septet length 39
(expected 39)
-Legacy encode case 3: septet length 40 (expected 40)
-Encode case 3: Octet length 35 (expected 35, computed 35), septet length 40
(expected 40)
-Legacy decode case 0: return value 9 (expected 9)
-Decode case 0: return value 9 (expected 9)
-Legacy decode case 1: return value 41 (expected 41)
-Decode case 1: return value 40 (expected 40)
-Legacy decode case 2: return value 39 (expected 39)
-Decode case 2: return value 31 (expected 31)
-Legacy decode case 3: return value 40 (expected 40)
-Decode case 3: return value 32 (expected 32)
-Decode case 4: return value 153 (expected 153)
-Decode case 5: return value 40 (expected 40)
+Encode case 0 (simple text): Octet length 8 (expected 8, computed 8, test OK),
septet length 9 (expected 9, test OK)
+Encode case 1 (escape text): Octet length 36 (expected 36, computed 36, test
OK), septet length 41 (expected 41, test OK)
+Encode case 2 (enhanced text): Octet length 35 (expected 35, computed 35, test
OK), septet length 39 (expected 39, test OK)
+Encode case 3 (enhanced text v2): Octet length 35 (expected 35, computed 35,
test OK), septet length 40 (expected 40, test OK)
+Decode case 0 (simple text): return value 9, expected 9, test OK
+Decode case 1 (escape text): return value 40, expected 40, test OK
+Decode case 2 (enhanced text): return value 31, expected 31, test OK
+Decode case 3 (enhanced text v2): return value 32, expected 32, test OK
+Decode case 4 (concatenated text p1): return value 153, expected 153, test OK
+Decode case 5 (concatenated text p2): return value 40, expected 40, test OK
Encoding some tests and printing number of septets/octets
SEPTETS: 8 OCTETS: 7
Done
--
2.5.0
