-
Notifications
You must be signed in to change notification settings - Fork 124
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
Fix use of errno
in SystemError
#455
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like the right thing to do. Should double check that there's no other instance that needs to be updated for close
.
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's a test case one could come up with to reproduce the original issue, verify that the change fixes it, and prevent future regressions?
I think we should continue to check for a return value of -1 before we consult |
Only return values of 0 and -1 are defined in POSIX. So what value we should be using instead for a theoretical -2 is unclear to me.
|
Calling
errno
insideSystemError.description()
is far too late and will almost always return the wrong error number. Instead, pass theerrno
at the throw site, which is consistent with how most of the otherSystemError
throws work.