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

Support sendmmsg/recvmmsg #1208

Merged
merged 2 commits into from
Apr 26, 2020
Merged

Support sendmmsg/recvmmsg #1208

merged 2 commits into from
Apr 26, 2020

Conversation

glebpom
Copy link
Contributor

@glebpom glebpom commented Apr 8, 2020

This pull request adds sendmmsg and recvmmsg functions. They are used to send and receive multiple messages per one call. Additionally, AsMut implemented for both TimeSpec and TimeVal.

This is still WIP, because we need to find the best way on how to pass multiple values to *mmsg functions. Current approach with iterators doesn't seem ergonomic.

@glebpom glebpom changed the title Support sendmmsg/recvmmsg [WIP] Support sendmmsg/recvmmsg Apr 8, 2020
@glebpom glebpom changed the title [WIP] Support sendmmsg/recvmmsg Support sendmmsg/recvmmsg Apr 11, 2020
@asomers
Copy link
Member

asomers commented Apr 19, 2020

Could you please fix the test failure before I review this PR?

Errno::result(ret).map(|r| r as usize)
}

#[cfg(any(target_os = "linux", target_os = "freebsd"))]
Copy link
Member

Choose a reason for hiding this comment

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

It looks like sendmmsg is also supported on Android, Dragonfly, NetBSD, and OpenBSD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will fix it. I didn't find the support on Dragonfly.

src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
src/sys/socket/mod.rs Outdated Show resolved Hide resolved
test/sys/test_socket.rs Outdated Show resolved Hide resolved
test/sys/test_socket.rs Outdated Show resolved Hide resolved
@asomers
Copy link
Member

asomers commented Apr 21, 2020

Looks like you have some test failures to fix.

@glebpom glebpom force-pushed the mmsg branch 2 times, most recently from b8373d2 to 77421b1 Compare April 21, 2020 11:38
{
let iter = data.into_iter();

let (min_size, max_size) = iter.size_hint();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about adding a restriction to force IntoIterator::IntoIter always be an ExactSizeIterator? Additionally, use set_len right after vector initialization instead of MaybeUninit magic. It will simplify the whole logic of Vectors initializations, and prevent possible allocations. @asomers

Copy link
Member

Choose a reason for hiding this comment

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

I think that's too great of a restriction. I don't know if I've seen a single Rust API that requires an ExactSizeIterator.

@asomers
Copy link
Member

asomers commented Apr 22, 2020

Oh, and please don't force-push until the end. It makes review harder.

src/sys/socket/mod.rs Outdated Show resolved Hide resolved
@asomers
Copy link
Member

asomers commented Apr 24, 2020

Looking a lot better. But it needs a more detailed review than my current level of sleepiness will allow. I'll set a reminder to do it tomorrow. Feel free to ping me if you don't hear anything for a few days.

@asomers
Copy link
Member

asomers commented Apr 25, 2020

@glebpom I understand now the need for ExactSizeIterator in recvvmsg, and it seems reasonable. But I made a change to remove a mem::zeroed. Could you please review it?

Copy link
Contributor Author

@glebpom glebpom left a comment

Choose a reason for hiding this comment

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

I was also thinking about something like this. Looks good to me, thanks!

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 26, 2020

Build succeeded:

@bors bors bot merged commit e61ae4a into nix-rust:master Apr 26, 2020
@glebpom glebpom deleted the mmsg branch April 28, 2020 08:20
nickpelone added a commit to nickpelone/nix that referenced this pull request May 26, 2020
… for sendmmsg() / recvmmsg() in nix-rust#1208.

In nix-rust#1208, sendmmsg() / recvmmsg() were added, but OpenBSD(who doesn't support these)
was included on the list of allowed operating systems for sendmmsg() related things.
This broke the build on OpenBSD.

For more Rust-world examples, see: rust-lang/libc@6f62973
bors bot added a commit that referenced this pull request May 31, 2020
1252: Add EV_DISPATCH, EV_RECEIPT items for EventFlags, and fix build on OpenBSD r=asomers a=nickpelone

This pull request:

- Adds `EV_DISPATCH` and `EV_RECEIPT` to `EventFlags`. OpenBSD supports these now. 
c379d1a
- Fixes a regression that caused `nix` to be unable to build on OpenBSD since #1208. 
dd0a990
- Makes a change to avoid a deprecation warning from `libc` crate, which breaks tests. 
0f9fcbd

Since I know y'all don't have OpenBSD in CI right now, I've attached the results of a `cargo test` run on OpenBSD 6.7.

Thanks for your time!

[testresults.txt](https://github.com/nix-rust/nix/files/4684597/testresults.txt)

Fixes #1251 

Co-authored-by: Nick Pelone <nick.pelone@calyptix.com>
bors bot added a commit that referenced this pull request May 31, 2020
1252: Add EV_DISPATCH, EV_RECEIPT items for EventFlags, and fix build on OpenBSD r=asomers a=nickpelone

This pull request:

- Adds `EV_DISPATCH` and `EV_RECEIPT` to `EventFlags`. OpenBSD supports these now. 
c379d1a
- Fixes a regression that caused `nix` to be unable to build on OpenBSD since #1208. 
dd0a990
- Makes a change to avoid a deprecation warning from `libc` crate, which breaks tests. 
0f9fcbd

Since I know y'all don't have OpenBSD in CI right now, I've attached the results of a `cargo test` run on OpenBSD 6.7.

Thanks for your time!

[testresults.txt](https://github.com/nix-rust/nix/files/4684597/testresults.txt)

Fixes #1251 

Co-authored-by: Nick Pelone <nick.pelone@calyptix.com>
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.

2 participants