Skip to content

Add an feof callback to Prism stream parsing#3602

Merged
tenderlove merged 3 commits into
ruby:mainfrom
tenderlove:eof-callback
Aug 4, 2025
Merged

Add an feof callback to Prism stream parsing#3602
tenderlove merged 3 commits into
ruby:mainfrom
tenderlove:eof-callback

Conversation

@tenderlove

Copy link
Copy Markdown
Member

Checking the return value of fgets isn't good enough to determine whether the input stream has reached EOF or not. fgets could have run in to an error and returned NULL. Additionally, we can't just check the buffer contents for \n because fgets could have read the maximum buffer size (which may not have ended with a \n).

I want to use this callback to implement a fix for:

https://bugs.ruby-lang.org/issues/21188

@kddnewton kddnewton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks right, though I think the FFI build is having issues.

@tenderlove

Copy link
Copy Markdown
Member Author

Looks right, though I think the FFI build is having issues.

Ya, I'm trying to figure out the FFI issues :|

@kddnewton

Copy link
Copy Markdown
Collaborator

Looks like we just need documentation on the new callback (https://github.com/ruby/prism/actions/runs/16603543403/job/46969381276?pr=3602#step:8:153). memcheck is an expected failure, and the test-all failure is telling us we need to merge this into ruby/ruby first.

tenderlove and others added 3 commits August 1, 2025 10:36
Checking the return value of `fgets` isn't good enough to determine
whether the input stream has reached EOF or not.  `fgets` could have run
in to an error and returned NULL.  Additionally, we can't just check the
buffer contents for `\n` because `fgets` could have read the maximum
buffer size (which may not have ended with a `\n`).

I want to use this callback to implement a fix for:

  https://bugs.ruby-lang.org/issues/21188
@tenderlove
tenderlove merged commit cfd4672 into ruby:main Aug 4, 2025
59 of 61 checks passed
@tenderlove
tenderlove deleted the eof-callback branch August 4, 2025 19:47
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