Skip to content
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.

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

Problematic assert() in Eio.File.pread #579

Closed
SGrondin opened this issue Jul 9, 2023 · 1 comment · Fixed by #581
Closed

Problematic assert() in Eio.File.pread #579

SGrondin opened this issue Jul 9, 2023 · 1 comment · Fixed by #581

Comments

@SGrondin
Copy link
Collaborator

SGrondin commented Jul 9, 2023

I might be wrong, but this assert appears misguided to me.

(* Current implementation of Eio.File.pread: *)

let pread (t : #ro) ~file_offset bufs =
  let got = t#pread ~file_offset bufs in
  assert (got > 0 && got <= Cstruct.lenv bufs);
  got

Imagine I'm reading a file slice by slice, using successive calls to Eio.File.pread.

When I reach the end of the file (i.e. when I pass a ~file_offset equal to the number of bytes in the file), Eio.File.pread returns a length of 0 and thus the assert fails.

It appears to me that the author expected t#pread to raise End_of_file to indicate EOF.

Either the Eio_posix implementation of Eio.File.ro#pread needs to raise End_of_file, or the assertion should be

assert (got <= Cstruct.lenv bufs);

I might be wrong here! but at the moment I'm having to circumvent Eio.File.pread by making direct calls to t#pread.

@talex5
Copy link
Collaborator

talex5 commented Jul 11, 2023

Yes, eio_posix needs to change (#581). The assert is here precisely to make sure that all the backends behave the same way (eio_linux already raises End_of_file here).

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 a pull request may close this issue.

2 participants