core: fix some bugs in GSM7 packing code

The existing gsm_pack() had a bug where if the last
septet should be in an octet by itself, that last
octet wouldn't be added.  Plus, kinda pointless to
use a GByteArray here when we already know what the
length will be through simple arithmetic.

We can also simplify the function too.

Furthermore, there weren't any testcases for starting
packing at an offset other than zero, so add one.

src/mm-charsets.c

@@ -672,37 +672,33 @@ gsm_pack (const guint8 *src,
           guint8 start_offset,
           guint32 *out_packed_len)
 {
-    GByteArray *packed;
+    guint8 *packed;
-    guint8 c, add_last = 0;
+    guint octet = 0, lshift, plen;
-    int i;
+    int i = 0;
 
-    packed = g_byte_array_sized_new (src_len);
+    g_return_val_if_fail (start_offset < 8, NULL);
 
-    for (i = 0, c = 0; i < src_len; i++) {
+    plen = (src_len * 7) + start_offset; /* total length in bits */
-        guint8 bits_here, offset;
+    if (plen % 8)
-        guint32 start_bit;
+        plen += 8;
+    plen /= 8;  /* now in bytes */
 
-        start_bit = start_offset + (i * 7); /* Overall bit offset of char in buffer */
+    packed = g_malloc0 (plen);
-        offset = start_bit % 8; /* Offset to start of char in this byte */
-        bits_here = offset ? (8 - offset) : 7;
 
-        c |= (src[i] & 0x7F) << offset;
+    for (i = 0, lshift = start_offset; i < src_len; i++) {
-        if (offset) {
+        packed[octet] |= (src[i] & 0x7F) << lshift;
-            /* Add this packed byte */
+        if (lshift > 1) {
-            g_byte_array_append (packed, &c, 1);
+            /* Grab the lost bits and add to next octet */
-            c = add_last = 0;
+            g_assert (octet + 1 < plen);
+            packed[octet + 1] = (src[i] & 0x7F) >> (8 - lshift);
+        }
+        if (lshift)
+            octet++;
+        lshift = lshift ? lshift - 1 : 7;
     }
 
-        /* Pack the rest of this char into the next byte */
+    if (out_packed_len)
-        if (bits_here != 7) {
+        *out_packed_len = plen;
-            c = (src[i] & 0x7F) >> bits_here;
+    return packed;
-            add_last = 1;
-        }
-    }
-    if (add_last)
-        g_byte_array_append (packed, &c, 1);
-
-    *out_packed_len = packed->len;
-    return g_byte_array_free (packed, FALSE);
 }
 

src/tests/test-charsets.c

@@ -284,6 +284,48 @@ test_pack_gsm7_24_chars (void *f, gpointer d)
     g_free (packed);
 }
 
+static void
+test_pack_gsm7_last_septet_alone (void *f, gpointer d)
+{
+    static const guint8 unpacked[] = {
+        0x54, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, 0x72, 0x65, 0x61, 0x6C,
+        0x6C, 0x79, 0x20, 0x63, 0x6F, 0x6F, 0x6C, 0x20, 0x10, 0x10, 0x10, 0x10,
+        0x10
+    };
+    static const guint8 expected[] = {
+        0x54, 0x74, 0x7A, 0x0E, 0x4A, 0xCF, 0x41, 0xF2, 0x72, 0x98, 0xCD, 0xCE,
+        0x83, 0xC6, 0xEF, 0x37, 0x1B, 0x04, 0x81, 0x40, 0x20, 0x10
+    };
+    guint8 *packed;
+    guint32 packed_len = 0;
+
+    /* Tests that a 25-character unpacked string (where, when packed, the last
+     * septet will be in an octet by itself) packs correctly.
+     */
+
+    packed = gsm_pack (unpacked, sizeof (unpacked), 0, &packed_len);
+    g_assert (packed);
+    g_assert_cmpint (packed_len, ==, sizeof (expected));
+
+    g_free (packed);
+}
+
+static void
+test_pack_gsm7_7_chars_offset (void *f, gpointer d)
+{
+    static const guint8 unpacked[] = { 0x68, 0x65, 0x6C, 0x6C, 0x6F, 0x10, 0x2F };
+    static const guint8 expected[] = { 0x00, 0x5D, 0x66, 0xB3, 0xDF, 0x90, 0x17 };
+    guint8 *packed;
+    guint32 packed_len = 0;
+
+    packed = gsm_pack (unpacked, sizeof (unpacked), 5, &packed_len);
+    g_assert (packed);
+    g_assert_cmpint (packed_len, ==, sizeof (expected));
+    g_assert_cmpint (memcmp (packed, expected, packed_len), ==, 0);
+
+    g_free (packed);
+}
+
 
 #if GLIB_CHECK_VERSION(2,25,12)
 typedef GTestFixtureFunc TCFunc;
@@ -314,6 +356,9 @@ int main (int argc, char **argv)
     g_test_suite_add (suite, TESTCASE (test_pack_gsm7_7_chars, NULL));
     g_test_suite_add (suite, TESTCASE (test_pack_gsm7_all_chars, NULL));
     g_test_suite_add (suite, TESTCASE (test_pack_gsm7_24_chars, NULL));
+    g_test_suite_add (suite, TESTCASE (test_pack_gsm7_last_septet_alone, NULL));
+
+    g_test_suite_add (suite, TESTCASE (test_pack_gsm7_7_chars_offset, NULL));
 
     result = g_test_run ();