Fix string_of_bitstring/add_bits handling of non-multiple-of-8-bit length strings...

diff --git a/bitmatch.ml b/bitmatch.ml
index 7065d66..1d4b30f 100644 (file)
--- a/bitmatch.ml
+++ b/bitmatch.ml
@@ -671,13 +671,15 @@ module Buffer = struct
           *)
          let slenbytes = slen lsr 3 in
          if slenbytes > 0 then Buffer.add_substring buf str 0 slenbytes;
-         t.last <- Char.code str.[slenbytes] lsl (8 - (slen land 7))
+         let last = Char.code str.[slenbytes] in (* last char *)
+         let mask = 0xff lsl (8 - (slen land 7)) in
+         t.last <- last land mask
        );
        t.len <- len + slen
       ) else (
        (* Target buffer is unaligned.  Copy whole bytes using
         * add_byte which knows how to deal with an unaligned
-        * target buffer, then call _add_bits for the remaining < 8 bits.
+        * target buffer, then call add_bit for the remaining < 8 bits.
         *
         * XXX This is going to be dog-slow.
         *)
@@ -686,7 +688,14 @@ module Buffer = struct
          let byte = Char.code str.[i] in
          add_byte t byte
        done;
-       _add_bits t (Char.code str.[slenbytes]) (slen - (slenbytes lsl 3))
+       let bitsleft = slen - (slenbytes lsl 3) in
+       if bitsleft > 0 then (
+         let c = Char.code str.[slenbytes] in
+         for i = 0 to bitsleft - 1 do
+           let bit = c land (0x80 lsr i) <> 0 in
+           add_bit t bit
+         done
+       )
       );
     )
 end
@@ -771,11 +780,8 @@ let string_of_bitstring (data, off, len) =
        str.[i] <- Char.chr c;
        loop data off len (i+1)
       ) else if len > 0 then (
-       (* XXX Is this correct?  It should write into the high bits
-        * of the last byte.
-        *)
-       let c, off, len = extract_char_unsigned data off len len in
-       str.[i] <- Char.chr c
+       let c, _, _ = extract_char_unsigned data off len len in
+       str.[i] <- Char.chr (c lsl (8-len))
       )
     in
     loop data off len 0;

diff --git a/tests/30_bitbuffer.ml b/tests/30_bitbuffer.ml
new file mode 100644 (file)
index 0000000..e9a95f9
--- /dev/null
+++ b/tests/30_bitbuffer.ml
@@ -0,0 +1,66 @@
+(* Test the Bitmatch.Buffer module and string_of_bitstring in
+ * nasty non-aligned corner cases.
+ * $Id$
+ *)
+
+open Printf
+
+let () =
+  Random.self_init ();
+
+  let str1 = "012345678" in
+
+  for offset = 0 to 65 do
+    for len = 1 to 65 do
+      let expected =
+       let strlen = (len+7) lsr 3 in
+       let expected = String.create strlen in
+       for i = 0 to strlen-1 do
+         expected.[i] <- Char.chr (Random.int 256)
+       done;
+       let last = Char.code expected.[strlen-1] in
+       let last = last land (0xff lsl (8 - (len land 7))) in
+       expected.[strlen-1] <- Char.chr last;
+       expected in
+
+      (* Create a random bitstring:
+       * +-------------+-------------------------------------------+
+       * | (random)    | bits that we check (expected)             |
+       * +-------------+-------------------------------------------+
+       * 0           offset                                    offset+len
+       *                <---------------- len bits --------------->
+       *)
+      let bits =
+       let bits = Bitmatch.Buffer.create () in
+       Bitmatch.Buffer.add_bits bits str1 offset;
+       Bitmatch.Buffer.add_bits bits expected len;
+       Bitmatch.Buffer.contents bits in
+
+      (* Create a sub bitstring corresponding to what we want to check. *)
+      let subbits =
+       let bits, bitoffset, bitlen = bits in
+       (bits, bitoffset+offset, bitlen-offset) in
+
+      assert (Bitmatch.bitstring_length subbits = len);
+
+      (* Now try to read out the substring using string_of_bitstring. *)
+      let actual = Bitmatch.string_of_bitstring subbits in
+      if actual <> expected then (
+       eprintf "MISMATCH between actual and expected, offset=%d, len=%d\n"
+         offset len;
+       eprintf "EXPECTED string:\n";
+       for i = 0 to String.length expected-1 do
+         eprintf " %02x" (Char.code expected.[i])
+       done;
+       eprintf "\nACTUAL string:\n";
+       for i = 0 to String.length actual-1 do
+         eprintf " %02x" (Char.code actual.[i])
+       done;
+       eprintf "\nBITS:\n";
+       Bitmatch.hexdump_bitstring stderr bits;
+       eprintf "SUBBITS:\n";
+       Bitmatch.hexdump_bitstring stderr subbits;
+       exit 1
+      );
+    done
+  done