-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
std.os: add support for socketpair #18561
Conversation
d81d7a5
to
2893e56
Compare
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.
some thoughts in no particular order
- nice!
- if windows doesn't have this behavior I don't think std.os should pretend it does imo.
- additionally it should accept
i32
orc_int
and not bitCast E.OPNOTSUPP, E.PROTONOSUPPORT
shouldn't be in the same case- neither should
E.NOBUFS, E.NOMEM
- there shouldn't be util functions, it should all be in the if statement of
socketpair
- common style for std.os is
if (windows) { _win_ } _posix_
, this is in the wrong order and shouldn't use anelse
2893e56
to
f43f2ad
Compare
Seems like
Which errors would be more appropriate?
They're separate functions because error.AddressFamilyNotSupported => return socketpairEmulatedAfInet(),
|
https://pubs.opengroup.org/onlinepubs/009696699/functions/socketpair.html |
.OPNOTSUPP => return error.OperationNotSupported,
.PROTONOSUPPORT => return error.ProtocolNotSupported,
.NOBUFS => return error.SystemResources,
.NOMEM => return error.SystemResources, |
e8778d1
to
c7c3fb3
Compare
done. why are we using separate switch prongs for the same error? |
c7c3fb3
to
edb1663
Compare
edb1663
to
9f03804
Compare
no particular reason afaik. just the style that core team/prs has kept up so far in this case to keep things vertical |
these test failures are looking pretty mysterious:
I don't see how my changes could possibly have caused this |
looks like master branch is failing too |
@@ -452,3 +452,5 @@ pub extern "kernel32" fn RegOpenKeyExW( | |||
) callconv(WINAPI) LSTATUS; | |||
|
|||
pub extern "kernel32" fn GetPhysicallyInstalledSystemMemory(TotalMemoryInKilobytes: *ULONGLONG) BOOL; | |||
|
|||
pub extern "kernel32" fn GetTempPathA(nBufferLength: DWORD, lpBuffer: LPSTR) callconv(WINAPI) DWORD; |
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.
no
fixes #18417
cc @nektro
with the
AF_UNIX
implementation, the file the socket is bound to is not cleaned up, it might be worth exposing this eventually or abstracting socketpair properly in std.net (related: #6600), comments are appreciated(also: fix an error where recvfrom would use
std.c
instead ofstd.os.windows
even when libc is not linked)