-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
sockmodule.c: sock_connect vs negative errno values... #84808
Comments
(Forgive any obvious mistakes in this report, I'm almost illiterate with Python, doubly so with Python internals.) In trying to get buildbot-worker running on Haiku ( https://haiku-os.org/ ), it runs into a situation where it tries to connect a non-blocking TCP socket, which correctly reports EINPROGRESS, and cpython/Modules/sockmodule.c's internal_connect() returns this error code to sock_connect() and sock_connect_ex(). Both of the sock_connect* functions will return NULL if the error code is negative, but on Haiku, all the errno values are negative (EINPROGRESS, for example, is -2147454940). I _think_ what sock_connect is intending to do here... res = internal_connect(s, SAS2SA(&addrbuf), addrlen, 1);
if (res < 0)
return NULL; ...is say "if we had a devastating and unexpected system error, give up immediately." Buildbot-worker seems to confirm this by throwing this exception in response: builtins.SystemError: <method 'connect_ex' of '_socket.socket' objects> returned NULL without setting an error internal_connect returns -1 in those devastating-and-unexpected cases--namely when an exception is to be raised--and does not ever use that to otherwise signify a legit socket error. Linux and other systems don't otherwise fall into this "res < 0" condition because errno values are positive on those systems. So I believe the correct fix here, in sock_connect() and sock_connect_ex(), is to check "if (res == -1)" instead of "res < 0" and let all other negative error codes carry on. If this seems like the correct approach, I can assemble a pull request, but I don't know the full ramifications of this small change, so I thought I'd report it here first. --ryan. |
Since Haiku is not a supported platform, I'm relabelling this from bug to feature request. I don't know how likely it is that it will be implemented though, there would need to be a core developer who cares enough about Haiku to do something non-standard for it (an assess what the implications of that may be). |
IMO, Python should not assume that errno codes are positive. The point of PEP-11 is to not maintain speedups/workarounds for niche platforms, not to overfit CPython to rely on quirks of the tiered platforms. Please send the PR. Considering the ramifications of the change is the reviewer's job. |
I agree with Petr; this definitely sounds like a bug, not a feature. |
Python shouldn't assume that errno codes are positive. reported and proposed by Ryan C. Gordon
Python shouldn't assume that errno codes are positive. reported and proposed by Ryan C. Gordon
internal_connect() errors when returning -1 with exception set. reported and proposed by Ryan C. Gordon
internal_connect() errors when returning -1 with exception set. reported and proposed by Ryan C. Gordon error handling scheme (raise, no raise) as suggested by encukou Co-authored-by: Petr Viktorin <encukou@gmail.com>
POSIX allows errno to be negative. Even though all currently supported platforms have non-negative errno, relying on a quirk like that would make Python less portable.
Thank you |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: