Skip to content

Fix indexing in some parsing helpers #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
avsm merged 5 commits into from
Sep 5, 2026
Merged

Fix indexing in some parsing helpers #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f67caf9
Add failing test cases.
avsm Sep 5, 2026
204eac8
Write filter_map results at the compacted index after dropped bytes.
avsm Sep 5, 2026
55d4f74
Remove only the final byte when taking a reverse tail.
avsm Sep 5, 2026
1c8271a
Compare reverse cuts with the separator and retain source offsets.
avsm Sep 5, 2026
84c1ce9
Include source offsets in find and find_sub results.
avsm Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions lib/cstruct.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ let head ?(rev= false) ({ len; _ } as cs) =

let tail ?(rev= false) ({ buffer; off; len; } as cs) =
if len = 0 then cs
else if rev then of_bigarray ~off ~len:(len - 2) buffer
else if rev then of_bigarray ~off ~len:(len - 1) buffer
else of_bigarray ~off:(off + 1) ~len:(len - 1) buffer

let is_empty { len; _ } = len = 0
Expand Down Expand Up @@ -816,23 +816,25 @@ let fcuts ~no_empty ~sep:({ len= sep_len; _ } as sep)
scan 0 0 []

let rcuts ~no_empty ~sep:({ len= sep_len; _ } as sep)
({ buffer; len; _ } as cs) =
({ buffer; off; len; } as cs) =
if sep_len = 0 then invalid_arg "cuts: empty separator" ;
let s_len = len in
let max_sep_zidx = sep_len - 1 in
let max_s_zidx = len - 1 in
let rec check_sep zanchor i k acc =
if k > max_sep_zidx
then let off = i + sep_len in
rscan i (i - sep_len) (add_sub ~no_empty buffer ~off ~len:(zanchor - off) acc)
then let start = i + sep_len in
rscan i (i - sep_len)
(add_sub ~no_empty buffer ~off:(off + start)
~len:(zanchor - start) acc)
else
if get_char cs (i + k) = get_char cs k
if get_char cs (i + k) = get_char sep k
then check_sep zanchor i (k + 1) acc
else rscan zanchor (i - 1) acc
and rscan zanchor i acc =
if i < 0 then
if zanchor = s_len then ( if no_empty && s_len = 0 then [] else [ cs ])
else add_sub ~no_empty buffer ~off:0 ~len:zanchor acc
else add_sub ~no_empty buffer ~off ~len:zanchor acc
else
if get_char cs i = get_char sep 0
then check_sep zanchor i 1 acc
Expand All @@ -858,20 +860,20 @@ let fields ?(empty= false) ?(is_sep= is_white) ({ buffer; off; len; } as cs) =
end in
loop (max_pos - 1) max_pos []

let ffind sat ({ buffer= v; len; _ } as cs) =
let ffind sat ({ buffer= v; off; len; } as cs) =
let max_idx = len - 1 in
let rec loop i =
if i > max_idx then None
else if sat (get_char cs i)
then Some (buffer ~off:i ~len:1 v)
then Some (buffer ~off:(off + i) ~len:1 v)
else loop (i + 1) in
loop 0

let rfind sat ({ buffer= v; len; _ } as cs) =
let rfind sat ({ buffer= v; off; len; } as cs) =
let rec loop i =
if i < 0 then None
else if sat (get_char cs i)
then Some (buffer ~off:i ~len:1 v)
then Some (buffer ~off:(off + i) ~len:1 v)
else loop (i - 1) in
loop (len - 1)

Expand All @@ -896,13 +898,15 @@ let ffind_sub ~sub:({ len= sub_len; _ } as sub) ({ buffer= v; off; len; } as cs)
else loop (i + 1) 0 in
loop 0 0

let rfind_sub ~sub:({ len= sub_len; _ } as sub) ({ buffer= v; len; _ } as cs) =
let rfind_sub ~sub:({ len= sub_len; _ } as sub)
({ buffer= v; off; len; } as cs) =
if sub_len > len then None
else
let max_zidx_sub = sub_len - 1 in
let rec loop i k =
if i < 0 then None
else if k > max_zidx_sub then Some (buffer v ~off:i ~len:sub_len)
else if k > max_zidx_sub
then Some (buffer v ~off:(off + i) ~len:sub_len)
else if k > 0
then ( if get_char sub k = get_char cs (i + k)
then loop i (k + 1)
Expand Down Expand Up @@ -940,7 +944,7 @@ let filter_map f ({ len; _ } as cs) =
then (if k = len then b else sub b 0 k)
else match f (get_char cs i) with
| Some chr ->
set_char b i chr ;
set_char b k chr ;
loop b (k + 1) (i + 1)
| None ->
loop b k (i + 1) in
Expand Down
103 changes: 102 additions & 1 deletion lib_test/parse.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ let tail =
empty_pos (tail @@ buffer ~off:2 ~len:0 abc) 2 ;
empty_pos (tail @@ buffer ~off:2 ~len:1 abc) 3 ;
empty_pos (tail @@ buffer ~off:3 ~len:0 abc) 3 ;
empty_pos (tail ~rev:true @@ string "") 0 ;
empty_pos (tail ~rev:true @@ buffer ~off:0 ~len:0 abc) 0 ;
empty_pos (tail ~rev:true @@ buffer ~off:0 ~len:1 abc) 0 ;
Alcotest.(check cstruct) "a" (tail ~rev:true @@ buffer ~off:0 ~len:2 abc) (string "a") ;
Alcotest.(check cstruct) "ab" (tail ~rev:true @@ buffer ~off:0 ~len:3 abc) (string "ab") ;
empty_pos (tail ~rev:true @@ buffer ~off:1 ~len:0 abc) 1 ;
empty_pos (tail ~rev:true @@ buffer ~off:1 ~len:1 abc) 1 ;
Alcotest.(check cstruct) "b" (tail ~rev:true @@ buffer ~off:1 ~len:2 abc) (string "b") ;
empty_pos (tail ~rev:true @@ buffer ~off:2 ~len:0 abc) 2 ;
empty_pos (tail ~rev:true @@ buffer ~off:2 ~len:1 abc) 2 ;
empty_pos (tail ~rev:true @@ buffer ~off:3 ~len:0 abc) 3 ;
;;

let is_empty =
Expand Down Expand Up @@ -486,7 +497,7 @@ let cuts =
Alcotest.check_raises "invalid" invalid_cuts_argument
(fun () -> ignore (cuts ~sep:"" "")) ;
Alcotest.check_raises "invalid" invalid_cuts_argument
(fun () -> ignore (cuts ~sep:"" "")) ;
(fun () -> ignore (cuts ~rev:true ~sep:"" "")) ;
Alcotest.(check ls) "0" (cuts ~empty:true ~sep:"," "") [""] ;
Alcotest.(check ls) "1" (cuts ~empty:false ~sep:"," "") [] ;
Alcotest.(check ls) "2" (cuts ~empty:true ~sep:"," "") [""];
Expand Down Expand Up @@ -552,6 +563,76 @@ let cuts =
Alcotest.(check ls) "57" (cuts ~empty:false ~sep:"aa" "aaaaa") ["a"];
Alcotest.(check ls) "58" (cuts ~empty:true ~sep:"aa" "aaaaaa") [""; ""; ""; ""];
Alcotest.(check ls) "59" (cuts ~empty:false ~sep:"aa" "aaaaaa") [];
Alcotest.(check ls) "rev 0" (cuts ~rev:true ~empty:true ~sep:"," "") [""] ;
Alcotest.(check ls) "rev 1" (cuts ~rev:true ~empty:false ~sep:"," "") [] ;
Alcotest.(check ls) "rev 2" (cuts ~rev:true ~empty:true ~sep:"," "") [""];

@dinosaure dinosaure Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doublon with L.566

Alcotest.(check ls) "rev 3" (cuts ~rev:true ~empty:false ~sep:"," "") [];

@dinosaure dinosaure Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doublon with L.567

Alcotest.(check ls) "rev 4" (cuts ~rev:true ~empty:true ~sep:"," ",") [""; ""];
Alcotest.(check ls) "rev 5" (cuts ~rev:true ~empty:false ~sep:"," ",") [];
Alcotest.(check ls) "rev 6" (cuts ~rev:true ~empty:true ~sep:"," ",,") [""; ""; ""];
Alcotest.(check ls) "rev 7" (cuts ~rev:true ~empty:false ~sep:"," ",,") [];
Alcotest.(check ls) "rev 8" (cuts ~rev:true ~empty:true ~sep:"," ",,,") [""; ""; ""; ""];
Alcotest.(check ls) "rev 9" (cuts ~rev:true ~empty:false ~sep:"," ",,,") [];
Alcotest.(check ls) "rev 10" (cuts ~rev:true ~empty:true ~sep:"," "123") ["123"];
Alcotest.(check ls) "rev 11" (cuts ~rev:true ~empty:false ~sep:"," "123") ["123"];
Alcotest.(check ls) "rev 12" (cuts ~rev:true ~empty:true ~sep:"," ",123") [""; "123"];
Alcotest.(check ls) "rev 13" (cuts ~rev:true ~empty:false ~sep:"," ",123") ["123"];
Alcotest.(check ls) "rev 14" (cuts ~rev:true ~empty:true ~sep:"," "123,") ["123"; ""];
Alcotest.(check ls) "rev 15" (cuts ~rev:true ~empty:false ~sep:"," "123,") ["123";];
Alcotest.(check ls) "rev 16" (cuts ~rev:true ~empty:true ~sep:"," "1,2,3") ["1"; "2"; "3"];
Alcotest.(check ls) "rev 17" (cuts ~rev:true ~empty:false ~sep:"," "1,2,3") ["1"; "2"; "3"];
Alcotest.(check ls) "rev 18" (cuts ~rev:true ~empty:true ~sep:"," "1, 2, 3") ["1"; " 2"; " 3"];
Alcotest.(check ls) "rev 19" (cuts ~rev:true ~empty:false ~sep:"," "1, 2, 3") ["1"; " 2"; " 3"];
Alcotest.(check ls) "rev 20" (cuts ~rev:true ~empty:true ~sep:"," ",1,2,,3,") [""; "1"; "2"; ""; "3"; ""];
Alcotest.(check ls) "rev 21" (cuts ~rev:true ~empty:false ~sep:"," ",1,2,,3,") ["1"; "2"; "3";];
Alcotest.(check ls) "rev 22" (cuts ~rev:true ~empty:true ~sep:"," ", 1, 2,, 3,")
[""; " 1"; " 2"; ""; " 3"; ""];
Alcotest.(check ls) "rev 23" (cuts ~rev:true ~empty:false ~sep:"," ", 1, 2,, 3,") [" 1"; " 2";" 3";];
Alcotest.(check ls) "rev 24" (cuts ~rev:true ~empty:true ~sep:"<>" "") [""];
Alcotest.(check ls) "rev 25" (cuts ~rev:true ~empty:false ~sep:"<>" "") [];
Alcotest.(check ls) "rev 26" (cuts ~rev:true ~empty:true ~sep:"<>" "<>") [""; ""];
Alcotest.(check ls) "rev 27" (cuts ~rev:true ~empty:false ~sep:"<>" "<>") [];
Alcotest.(check ls) "rev 28" (cuts ~rev:true ~empty:true ~sep:"<>" "<><>") [""; ""; ""];
Alcotest.(check ls) "rev 29" (cuts ~rev:true ~empty:false ~sep:"<>" "<><>") [];
Alcotest.(check ls) "rev 30" (cuts ~rev:true ~empty:true ~sep:"<>" "<><><>") [""; ""; ""; ""];
Alcotest.(check ls) "rev 31" (cuts ~rev:true ~empty:false ~sep:"<>" "<><><>") [];
Alcotest.(check ls) "rev 32" (cuts ~rev:true ~empty:true ~sep:"<>" "123") [ "123" ];
Alcotest.(check ls) "rev 33" (cuts ~rev:true ~empty:false ~sep:"<>" "123") [ "123" ];
Alcotest.(check ls) "rev 34" (cuts ~rev:true ~empty:true ~sep:"<>" "<>123") [""; "123"];
Alcotest.(check ls) "rev 35" (cuts ~rev:true ~empty:false ~sep:"<>" "<>123") ["123"];
Alcotest.(check ls) "rev 36" (cuts ~rev:true ~empty:true ~sep:"<>" "123<>") ["123"; ""];
Alcotest.(check ls) "rev 37" (cuts ~rev:true ~empty:false ~sep:"<>" "123<>") ["123"];
Alcotest.(check ls) "rev 38" (cuts ~rev:true ~empty:true ~sep:"<>" "1<>2<>3") ["1"; "2"; "3"];
Alcotest.(check ls) "rev 39" (cuts ~rev:true ~empty:false ~sep:"<>" "1<>2<>3") ["1"; "2"; "3"];
Alcotest.(check ls) "rev 40" (cuts ~rev:true ~empty:true ~sep:"<>" "1<> 2<> 3") ["1"; " 2"; " 3"];
Alcotest.(check ls) "rev 41" (cuts ~rev:true ~empty:false ~sep:"<>" "1<> 2<> 3") ["1"; " 2"; " 3"];
Alcotest.(check ls) "rev 42" (cuts ~rev:true ~empty:true ~sep:"<>" "<>1<>2<><>3<>")
[""; "1"; "2"; ""; "3"; ""];
Alcotest.(check ls) "rev 43" (cuts ~rev:true ~empty:false ~sep:"<>" "<>1<>2<><>3<>") ["1"; "2";"3";];
Alcotest.(check ls) "rev 44" (cuts ~rev:true ~empty:true ~sep:"<>" "<> 1<> 2<><> 3<>")
[""; " 1"; " 2"; ""; " 3";""];
Alcotest.(check ls) "rev 45" (cuts ~rev:true ~empty:false ~sep:"<>" "<> 1<> 2<><> 3<>")[" 1"; " 2"; " 3"];
Alcotest.(check ls) "rev 46" (cuts ~rev:true ~empty:true ~sep:"<>" ">>><>>>><>>>><>>>>")
[">>>"; ">>>"; ">>>"; ">>>" ];
Alcotest.(check ls) "rev 47" (cuts ~rev:true ~empty:false ~sep:"<>" ">>><>>>><>>>><>>>>")
[">>>"; ">>>"; ">>>"; ">>>" ];
Alcotest.(check ls) "rev 48" (cuts ~rev:true ~empty:true ~sep:"<->" "<->>->") [""; ">->"];
Alcotest.(check ls) "rev 49" (cuts ~rev:true ~empty:false ~sep:"<->" "<->>->") [">->"];
Alcotest.(check ls) "rev 50" (cuts ~rev:true ~empty:true ~sep:"aa" "aa") [""; ""];
Alcotest.(check ls) "rev 51" (cuts ~rev:true ~empty:false ~sep:"aa" "aa") [];
Alcotest.(check ls) "rev 52" (cuts ~rev:true ~empty:true ~sep:"aa" "aaa") ["a"; ""];
Alcotest.(check ls) "rev 53" (cuts ~rev:true ~empty:false ~sep:"aa" "aaa") ["a"];
Alcotest.(check ls) "rev 54" (cuts ~rev:true ~empty:true ~sep:"aa" "aaaa") [""; ""; ""];
Alcotest.(check ls) "rev 55" (cuts ~rev:true ~empty:false ~sep:"aa" "aaaa") [];
Alcotest.(check ls) "rev 56" (cuts ~rev:true ~empty:true ~sep:"aa" "aaaaa") ["a"; ""; ""];
Alcotest.(check ls) "rev 57" (cuts ~rev:true ~empty:false ~sep:"aa" "aaaaa") ["a"];
Alcotest.(check ls) "rev 58" (cuts ~rev:true ~empty:true ~sep:"aa" "aaaaaa") [""; ""; ""; ""];
Alcotest.(check ls) "rev 59" (cuts ~rev:true ~empty:false ~sep:"aa" "aaaaaa") [];
let positions =
Cstruct.cuts ~rev:true ~empty:true ~sep:(s ",") (s "a,")
|> List.map (fun cs -> start_pos cs, stop_pos cs)
in
Alcotest.(check (list (pair int int))) "64" positions [(1, 2); (3, 3)];
;;

let fields =
Expand Down Expand Up @@ -621,6 +702,17 @@ let find =
Alcotest.(check (option cstruct)) "7" (find ~rev:true (fun c -> c = 'b') abcbd) (Some b1);
Alcotest.(check (option cstruct)) "8" (find (fun c -> c = 'b') ab) (Some b0);
Alcotest.(check (option cstruct)) "9" (find ~rev:true (fun c -> c = 'b') ab) (Some b0);
let { Cstruct.buffer= padded; _ } = Cstruct.of_string "_abcbd_" in
let shifted = buffer ~off:1 ~len:5 padded in
let check_shifted label expected_pos result = match result with
| None -> Alcotest.failf "%s: expected a match" label
| Some result ->
Alcotest.(check char) label (get_char result 0) 'b';
Alcotest.(check int) label (start_pos result) expected_pos
in
check_shifted "offset forward" 2 (find (fun c -> c = 'b') shifted);
check_shifted "offset reverse" 4
(find ~rev:true (fun c -> c = 'b') shifted);
;;

let find_sub =
Expand All @@ -644,6 +736,15 @@ let find_sub =
Alcotest.(check (option cstruct)) "9" (find_sub ~rev:true ~sub:(Cstruct.of_string "b") abcbd) (Some b1);
Alcotest.(check (option cstruct)) "10" (find_sub ~sub:b1 ab) (Some b0);
Alcotest.(check (option cstruct)) "11" (find_sub ~rev:true ~sub:b1 ab) (Some b0);
let { Cstruct.buffer= padded; _ } = Cstruct.of_string "_abcbd_" in
let shifted = buffer ~off:1 ~len:5 padded in
let result = find_sub ~rev:true ~sub:(string "bd") shifted in
Alcotest.(check (option cstruct)) "offset reverse" result (Some (string "bd"));
let result_pos = Option.map start_pos result in
Alcotest.(check (option int)) "offset reverse position" result_pos (Some 4);
let result = find_sub ~rev:true ~sub:(string "") shifted in
let result_pos = Option.map start_pos result in
Alcotest.(check (option int)) "offset reverse empty position" result_pos (Some 6);
;;

let () = Alcotest.run "cstruct.parse"
Expand Down
13 changes: 13 additions & 0 deletions lib_test/tests.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,15 @@ let rev_len_5 () =
let expected = Cstruct.of_string "edcba" in
assert_cs_equal expected (Cstruct.rev cs)

let filter_map_drops () =
let cs = Cstruct.of_string "banana" in
let fm = Cstruct.filter_map (fun c -> if c = 'a' then None else Some c) cs in
assert_cs_equal (Cstruct.of_string "bnn") fm

let filter_map_keeps_all () =
let cs = Cstruct.of_string "banana" in
assert_cs_equal cs (Cstruct.filter_map (fun c -> Some c) cs)

let test_hexdump ?(format=("%a" : _ format4)) cs expected =
let got = Format.asprintf format Cstruct.hexdump_pp cs in
Alcotest.(check string) "hexdump output" expected got
Expand Down Expand Up @@ -246,6 +255,10 @@ let suite = [
"len = 1", `Quick, rev_len_1;
"len = 5", `Quick, rev_len_5;
];
"filter_map", [
"drops bytes", `Quick, filter_map_drops;
"keeps all bytes", `Quick, filter_map_keeps_all;
];
"hexdump", [
"empty", `Quick, hexdump_empty;
"small", `Quick, hexdump_small;
Expand Down
Loading