Skip to content

Fix indexing in some parsing helpers - #324

Merged
avsm merged 5 commits into
mirage:mainfrom
avsm:fixes
Sep 5, 2026
Merged

avsm merged 5 commits into
mirage:mainfrom
avsm:fixes

Conversation

@avsm

@avsm avsm commented Sep 5, 2026

Copy link
Copy Markdown
Member

This corrects indexing and offset handling in filter_map, reverse tail and cuts, and find/find_sub on subviews.

It also adds regression coverage for forward and reverse parsing behavior and returned view offsets. Reviewed also by @samoht

avsm and others added 5 commits September 5, 2026 08:42
These cases reproduce each incorrect result before the implementation changes.

Co-authored-by: Thomas Gazagnaire <thomas@gazagnaire.org>
Writing at the compacted index avoids gaps when earlier bytes are dropped.
Reducing the view by one removes only its final byte and keeps singleton tails valid.
Using the separator and absolute view offsets returns the correct pieces from reverse scans.
Adding the source view offset points each returned slice at the matched bytes.
@samoht

samoht commented Sep 5, 2026

Copy link
Copy Markdown
Member

LGTM

@avsm
avsm merged commit 936f101 into mirage:main Sep 5, 2026
15 of 17 checks passed
jmid pushed a commit to ocaml/opam-repository that referenced this pull request Sep 6, 2026
CHANGES:

- Fix indexing and subview offset handling in `filter_map`,
  reverse `tail` and `cuts`, and `find`/`find_sub` (@avsm, @samoht, mirage/ocaml-cstruct#324)
Comment thread lib_test/parse.ml
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 •

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

Comment thread lib_test/parse.ml
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:"," "") [""];
Alcotest.(check ls) "rev 3" (cuts ~rev:true ~empty:false ~sep:"," "") [];

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants