Skip to content

make IO::Handle::error report errors from both input and output streams #17781

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

Merged
merged 3 commits into from
Jul 30, 2020

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented May 12, 2020

The perl IO SV has slots for an input PerlIO object and an output PerlIO object.

For normal files this is always the same PerlIO object, but for character devices and sockets a second PerlIO object is created for the same fd and stored in the output slot, this allows input and output to be buffered separately.

The IO::Handle::error() method however only checks the input handle, so if there's an error writing to a socket or character device that isn't reflected in the result of calling error() on the handle.

Similarly IO::Handle::clearerr() only clears the error/eof state on the input handle, but for character devices and sockets this meant only half the error state was reset.

This PR adjusts those methods to work on both the input and output streams if they're different.

tonycoz added 3 commits May 12, 2020 10:53
For character devices and sockets perl uses separate PerlIO objects
for input and output so they can be buffered separately.

The IO::Handle::error() method only checked the input stream, so
if a write error occurs error() would still returned false.

Change this so both the input and output streams are checked.

fixes Perl#6799
Similarly to GH Perl#6799 clearerr() only cleared the error status
of the input stream, so clear both.
and update the ChangeLog
@toddr toddr added this to the 5.33.1 milestone May 19, 2020
@toddr
Copy link
Member

toddr commented May 19, 2020

@tonycoz does this merge close #6799 ?

@tonycoz
Copy link
Contributor Author

tonycoz commented May 20, 2020

Yes it would fix #6799 (I believe the note in the first commit would trigger github's automation if this is merged.)

We're in a freeze, so I haven't merged it.

@xsawyerx xsawyerx added the do not merge Don't merge this PR, at least for now label Jun 20, 2020
@Leont
Copy link
Contributor

Leont commented Jun 21, 2020

Looks good to me

@Leont Leont removed the do not merge Don't merge this PR, at least for now label Jun 21, 2020
@khwilliamson khwilliamson merged commit d1c218b into Perl:blead Jul 30, 2020
@tonycoz tonycoz deleted the 6799-io-error branch August 3, 2020 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants