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.

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

Jump to bottom

Remove redundant addclose calls #435

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Remove redundant addclose calls #435

merged 1 commit into from
Nov 7, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Nov 6, 2023

We shouldn't need to use addclose on the file descriptors we're closing ourselves explicitly.

@neonichu
Remove redundant addclose calls
c8b69c2 
We shouldn't need to use addclose on the file descriptors we're closing ourselves explicitly.
@neonichu neonichu self-assigned this Nov 6, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Nov 6, 2023

@swift-ci please test

@tomerd
Copy link
Contributor

tomerd commented Nov 6, 2023 •
edited
Loading

what is the difference between stderr and stdout in this context?

I see it now

@neonichu
Copy link
Contributor Author

neonichu commented Nov 6, 2023

@swift-ci please test windows

2 similar comments
@neonichu
Copy link
Contributor Author

neonichu commented Nov 6, 2023

@swift-ci please test windows

@shahmishal
Copy link
Member

@swift-ci please test windows

@shahmishal
Copy link
Member

This should fix the windows test issue - swiftlang/swift#69689

@shahmishal shahmishal merged commit cf8a8a0 into swiftlang:main Nov 7, 2023
2 of 3 checks passed
@shahmishal
Copy link
Member

Merged to unblock CI bots

@neonichu neonichu deleted the remove-redundant-addclose branch November 7, 2023 16:09
grynspan added a commit that referenced this pull request Nov 7, 2023
@grynspan
Fix to #435 (wrong stdin fd closed.)
c19d749 
(With apologies to @neonichu.)

The previous commit closes the wrong file descriptor for `stdin`, which should close the duplicated _read_ descriptor (whereas `stdout` and `stderr` duplicate their _write_ descriptors.) This PR fixes that.
grynspan added a commit that referenced this pull request Nov 8, 2023
@grynspan
Revert "Remove redundant addclose calls (#435)"
a305dda 
This reverts commit cf8a8a0.
@grynspan grynspan mentioned this pull request Nov 8, 2023
Merged
tomerd pushed a commit that referenced this pull request Nov 8, 2023
@grynspan
Revert "Remove redundant addclose calls (#435)" (#438)
4d539ff 
This reverts commit cf8a8a0.
grynspan pushed a commit that referenced this pull request Nov 9, 2023
@neonichu @grynspan
Remove redundant addclose calls (#435)
0dc3ba4 
We shouldn't need to use addclose on the file descriptors we're closing ourselves explicitly.
grynspan added a commit that referenced this pull request Nov 9, 2023
@grynspan
Fix to #435 (wrong stdin fd closed.)
7b80249 
(With apologies to @neonichu.)

The previous commit closes the wrong file descriptor for `stdin`, which should close the duplicated _read_ descriptor (whereas `stdout` and `stderr` duplicate their _write_ descriptors.) This PR fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grynspan grynspan grynspan approved these changes

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

@abertelrud abertelrud Awaiting requested review from abertelrud

@tomerd tomerd Awaiting requested review from tomerd

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@neonichu @tomerd @shahmishal @MaxDesiatov @grynspan