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

Initial basic cmsg support for unix #316

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

brunowonka
Copy link

Also added set_recv_ttl while I'm here to use in testing. The other RECVxxx-type socket options might make more sense with control message support now.

I went back and forth on the shape of the public API for this quite a bit, and I also don't have access to a Windows machine to check.

Fixes #313

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

I'm afraid this doesn't work at all on a number of platform, please see the CI failure. I'll do another round of review once those are fixed.

src/socket.rs Outdated Show resolved Hide resolved
src/socket.rs Outdated Show resolved Hide resolved
src/socket.rs Outdated Show resolved Hide resolved
use std::iter::FromIterator;

#[derive(Debug, Clone)]
struct MsgHdrWalker<B> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove the generic parameter, we only need &[u8], same for the other structures.

Copy link
Author

Choose a reason for hiding this comment

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

Tentatively keeping this parameter so we can reuse some logic between &[u8] and &mut [u8], but removed all the type parameters from the public API for now.

src/sys/unix.rs Outdated Show resolved Hide resolved
src/sys/unix.rs Outdated Show resolved Hide resolved
src/sys/unix.rs Outdated Show resolved Hide resolved
src/sys/unix.rs Outdated Show resolved Hide resolved
src/sys/unix.rs Outdated Show resolved Hide resolved
@brunowonka
Copy link
Author

Thank you for looking, I'm working through the other platform problems here and I'll push as soon as I have things working so you can look through.

src/cmsg.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

Not a complete review, but a start.

tests/socket.rs Show resolved Hide resolved
src/socket.rs Outdated Show resolved Hide resolved
src/socket.rs Show resolved Hide resolved
src/socket.rs Show resolved Hide resolved
src/socket.rs Outdated Show resolved Hide resolved
- Split recv_msg from recv_msg_from.
- Introduce CmsgBuffer type.
@darconeous
Copy link

Looking at this failure:

Run cargo install cargo-hack
    Updating crates.io index
 Downloading crates ...
  Downloaded cargo-hack v0.5.15
error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec[8](https://github.com/rust-lang/socket2/runs/7569907645?check_suite_focus=true#step:4:9)23/cargo-hack-0.5.15/Cargo.toml`

Caused by:
  failed to parse the `edition` key

Caused by:
  this version of Cargo is older than the `2021` edition, and only supports `2015` and `2018` editions.
Error: Process completed with exit code [10](https://github.com/rust-lang/socket2/runs/7569907645?check_suite_focus=true#step:4:11)1.

Looks unrelated to this change?

The macOS failure looks real though:

running 36 tests
...snip...
test send_from_recv_to_vectored ... FAILED
test send_recv_vectored ... FAILED
...snip...

failures:
error: test failed, to rerun pass '--test socket'

error: process didn't exit successfully: `/Users/runner/.rustup/toolchains/stable-x86_64-apple-darwin/bin/cargo test --manifest-path Cargo.toml --no-default-features` (exit status: 101)
---- send_from_recv_to_vectored stdout ----
thread 'send_from_recv_to_vectored' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }', tests/socket.rs:[57](https://github.com/rust-lang/socket2/runs/7569907933?check_suite_focus=true#step:5:58)4:10

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.

Support ancillary data over sendmsg, recvmsg
4 participants