minor fixes, add tests

dune

@@ -2,3 +2,8 @@
   (name ohex)
   (public_name ohex)
   (modules ohex))
+
+(test
+  (name tests)
+  (modules tests)
+  (libraries alcotest ohex))

ohex.ml

@@ -8,25 +8,32 @@ let is_space = function
   | ' ' | '\n' | '\r' | '\t' -> true
   | _ -> false
 
-let count_hex_chars ?(skip_whitespace = true) src =
+let digit = function
-  if skip_whitespace then
+  | '0'..'9' as c -> int_of_char c - 0x30
+  | 'A'..'F' as c -> int_of_char c - 0x41 + 10
+  | 'a'..'f' as c -> int_of_char c - 0x61 + 10
+  | _ -> invalid_arg "bad character"
+
+let required_length ?(skip_whitespace = true) src =
+  let req =
     string_fold (fun r c ->
-      if is_space c then r else succ r)
+        if skip_whitespace && is_space c then
-    0 src / 2
+          r
+        else (
+          ignore (digit c);
+          succ r))
+      0 src
+  in
+  if req mod 2 = 0 then
+    req / 2
   else
-    String.length src / 2
+    invalid_arg "leftover byte in hex string"
 
 let decode_into ?(skip_whitespace = true) src tgt ?(off = 0) () =
   let fold f acc str =
     let st = ref acc in
     String.iter (fun c -> st := f !st c) str;
     !st
-  and digit c =
-    match c with
-    | '0'..'9' -> int_of_char c - 0x30
-    | 'A'..'F' -> int_of_char c - 0x41 + 10
-    | 'a'..'f' -> int_of_char c - 0x61 + 10
-    | _ -> invalid_arg "bad character"
   in
   let chars, leftover =
     fold (fun (chars, leftover) c ->
@@ -45,19 +52,21 @@ let decode_into ?(skip_whitespace = true) src tgt ?(off = 0) () =
   List.iteri (fun idx c -> Bytes.set_uint8 tgt (off + idx) c) chars
 
 let decode ?(skip_whitespace = true) src =
-  let len = count_hex_chars ~skip_whitespace src in
+  let len = required_length ~skip_whitespace src in
   let buf = Bytes.create len in
   decode_into ~skip_whitespace src buf ();
   Bytes.unsafe_to_string buf
 
+let hex_map = "0123456789abcdef"
+
 let encode_into src tgt ?(off = 0) () =
   String.iteri (fun idx c ->
       let hi, lo =
         let i = int_of_char c in
-        i lsr 4, i land 0xFF
+        hex_map.[i lsr 4], hex_map.[i land 0x0F]
       in
-      Bytes.set_uint8 tgt (idx * 2 + off) hi;
+      Bytes.set tgt (idx * 2 + off) hi;
-      Bytes.set_uint8 tgt (idx * 2 + off + 1) lo)
+      Bytes.set tgt (idx * 2 + off + 1) lo)
     src
 
 let encode src =

ohex.mli

@@ -1,10 +1,14 @@
-(** Convert from and to hex representation. *)
+(** Convert from and to hexadecimal representation. *)
 
-val count_hex_chars : ?skip_whitespace:bool -> string -> int
+val required_length : ?skip_whitespace:bool -> string -> int
-(** [count_hex_chars ~skip_whitespace s] counts the amount of hex characters in
+(** [required_length ~skip_whitespace s] returns the length needed when the
-    the string [s]. The argument [skip_whitespace] defaults to [true], and skips
+    hex string [s] would be decoded into a sequence of octets. The argument
-    any whitespace characters (' ', '\n', '\r', '\t'). This function is useful
+    [skip_whitespace] defaults to [true], and skips any whitespace characters
-    for estimating the space required for [decode_into]. *)
+    (' ', '\n', '\r', '\t'). This function is useful for estimating the space
+    required for [decode_into].
+
+    @raise Invalid_argument if any character in [s] is not a hex character, or
+    an odd amount of characters are present. *)
 
 val decode : ?skip_whitespace:bool -> string -> string
 (** [decode ~skip_whitespace s] decodes a hex string [s] into a sequence of

tests.ml

@@ -0,0 +1,85 @@
+
+let tests = [
+  "", 0, "";
+  "41", 1, "A";
+  "41 41", 2, "AA";
+  " 41 41 ", 2, "AA";
+  "   414  1", 2, "AA";
+]
+
+let len_dec_tests =
+  List.mapi (fun i (s, len, v) ->
+      string_of_int i ^ " is correct", `Quick,
+      (fun () ->
+         Alcotest.(check int "required length" len (Ohex.required_length s));
+         Alcotest.(check string "decode works fine" v (Ohex.decode s))))
+    tests
+
+let bad_char_input = [ "W" ; "AAWW" ; "WWAA" ]
+
+let leftover_input = [ "AAA" ; "A" ]
+
+let bad_input_ws = [ " "; " AA" ; "AA " ; "A A" ]
+
+let bad_len_dec_tests =
+  (List.mapi (fun i s ->
+       string_of_int i ^ " fails (bad character)", `Quick,
+       (fun () ->
+          Alcotest.(check_raises "required length raises"
+                      (Invalid_argument "bad character")
+                      (fun () -> ignore (Ohex.required_length s)));
+          Alcotest.(check_raises "decode raises"
+                      (Invalid_argument "bad character")
+                      (fun () -> ignore (Ohex.decode s)))))
+      bad_char_input) @
+  (List.mapi (fun i s ->
+       string_of_int i ^ " fails (leftover)", `Quick,
+       (fun () ->
+          Alcotest.(check_raises "required length raises"
+                      (Invalid_argument "leftover byte in hex string")
+                      (fun () -> ignore (Ohex.required_length ~skip_whitespace:false s)));
+          Alcotest.(check_raises "decode raises"
+                      (Invalid_argument "leftover byte in hex string")
+                      (fun () -> ignore (Ohex.decode ~skip_whitespace:false s)))))
+      leftover_input) @
+  (List.mapi (fun i s ->
+       string_of_int i ^ " fails (skip_whitespace = false)", `Quick,
+       (fun () ->
+          Alcotest.(check_raises "required length raises"
+                      (Invalid_argument "bad character")
+                      (fun () -> ignore (Ohex.required_length ~skip_whitespace:false s)));
+          Alcotest.(check_raises "decode raises"
+                      (Invalid_argument "bad character")
+                      (fun () -> ignore (Ohex.decode ~skip_whitespace:false s)))))
+      bad_input_ws)
+
+let dec_enc () =
+  let random_string () =
+    let size = Random.int 128 in
+    let buf = Bytes.create size in
+    for i = 0 to size - 1 do
+      Bytes.set_uint8 buf i (Random.int 256)
+    done;
+    Bytes.unsafe_to_string buf
+  in
+  for i = 0 to 10_000 do
+    let input = random_string () in
+    Alcotest.(check string ("dec (enc s) = s " ^ string_of_int i)
+                input Ohex.(decode (encode input)));
+    Alcotest.(check string ("dec ~skip_ws:false (enc s) = s " ^ string_of_int i)
+                input Ohex.(decode ~skip_whitespace:false (encode input)));
+    let buf = Bytes.create (String.length input * 2) in
+    Ohex.encode_into input buf ~off:0 ();
+    let out = Bytes.create (String.length input) in
+    Ohex.decode_into (Bytes.unsafe_to_string buf) out ~off:0 ();
+    Alcotest.(check string ("dec_into (enc_into s) = s " ^ string_of_int i)
+                input (Bytes.unsafe_to_string out))
+  done
+
+let suites = [
+  "length and decode pass", len_dec_tests ;
+  "bad input", bad_len_dec_tests ;
+  "decode encode", [ "decode (encode s) = s", `Quick, dec_enc ];
+]
+
+let () = Alcotest.run "hex tests" suites