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.

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

feat: Add sendmsg/recvmsg #505

Merged
merged 16 commits into from
Apr 28, 2023
Merged

Conversation

notgull
Copy link
Contributor

@notgull notgull commented Jan 8, 2023

This PR adds the sendmsg_*/recvmsg family of functions. It also adds structs for dealing with ancillary data. Some of the code here is taken from #159.

Very WIP for now

@chrysn
Copy link

chrysn commented Jan 31, 2023

Great to see this come, recvmsg would make this a potential backend for the async variety of the std-embedded-nal crate.

Given that this is in net, and that advertises Winsock2 support, and that there is some recvmsg-like stuff in the Windows APIs (eg. IN_RECVERR, IN_PKTINFO): Do you figure this could support recvmsg even on Windows?

@notgull notgull mentioned this pull request Apr 18, 2023
17 tasks
@valpackett
Copy link
Contributor

I'm interested in this, I'd like to add SCM_CREDENTIALS/SCM_CREDS support as well.

@notgull
Copy link
Contributor Author

notgull commented Apr 27, 2023

Re: both comments above, my motivating use case for this PR is using rustix in x11rb and wayland-rs, which both use sendmsg/recvmsg as a transport for sending/receiving file descriptors over a Unix socket. Therefore, I'm only really familiar with SCM_RIGHTS; I'm not familiar with other types of ancillary data.

Since the SendAncillaryMessage and RecvAncillaryMessage enums are both #[non_exhaustive], it shouldn't be too hard to add new ancillary data to the enum.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I've now made a preliminary pass with a few comments.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Show resolved Hide resolved
src/net/send_recv/msg.rs Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've now read through the whole PR. Overall this looks great!

src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/backend/linux_raw/net/msghdr.rs Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/backend/linux_raw/net/syscalls.rs Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/backend/libc/conv.rs Show resolved Hide resolved
src/backend/libc/conv.rs Outdated Show resolved Hide resolved
src/backend/libc/conv.rs Outdated Show resolved Hide resolved
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for one more round of review here, but I did find a few more things here.

src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
src/net/send_recv/msg.rs Outdated Show resolved Hide resolved
@notgull
Copy link
Contributor Author

notgull commented Apr 28, 2023

I apologize for one more round of review here, but I did find a few more things here.

No problem! Especially for a crate like rustix it's good to dot all our I's and cross all our T's.

@sunfishcode
Copy link
Member

Thanks!

@sunfishcode sunfishcode merged commit 2cdbb1d into bytecodealliance:main Apr 28, 2023
@notgull notgull deleted the ancillary branch April 28, 2023 22:11
@sunfishcode
Copy link
Member

This is now released in rustix 0.37.16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants