From: Richard W.M. Jones <rich@annexia.org>
Date: Sun, 18 May 2008 22:28:41 +0000 (+0000)
Subject: Fix string_of_bitstring/add_bits handling of non-multiple-of-8-bit length strings... 
X-Git-Url: http://git.annexia.org/?a=commitdiff_plain;h=7911c11ca6a9aefa8e5398ff0f39e74aaf0ac0cb;p=ocaml-bitstring.git

Fix string_of_bitstring/add_bits handling of non-multiple-of-8-bit length strings, and add a test case.
---

diff --git a/bitmatch.ml b/bitmatch.ml
index 7065d66..1d4b30f 100644
--- 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
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