Skip to content

Rely on libc for more socket constants #636

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jul 3, 2017
Merged

Conversation

Susurrus
Copy link
Contributor

Not all values are upstreamed, but this covers the vast majority of them.

pub const TCP_KEEPALIVE: c_int = libc::TCP_KEEPALIVE;
#[cfg(target_os = "freebsd")]
#[cfg(target_os = "freebsd", target_os = "netbsd")]
Copy link
Member

Choose a reason for hiding this comment

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

Missing an any( expression in your #[cfg. Also, libc has not defined TCP_KEEPIDLE for NetBSD. Only for FreeBSD, Dragonfly, Linux, Android, and Solaris.

pub const IPPROTO_IPV6: c_int = 41;
pub const IPPROTO_TCP: c_int = 6;
pub const IPPROTO_UDP: c_int = 17;
pub const SOL_SOCKET: c_int = libc::SOCK_SOCKET;
Copy link
Member

Choose a reason for hiding this comment

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

should this be libc::SOL_SOCKET`?

@Susurrus Susurrus force-pushed the constants branch 3 times, most recently from 2fa4eea to b2a0b40 Compare June 29, 2017 18:36
@Susurrus Susurrus changed the title Rely on libc for more signal constants Rely on libc for more socket constants Jun 29, 2017
@asomers
Copy link
Member

asomers commented Jun 30, 2017

I still can't find the SOCK_SOCKET symbol anywhere. Are you sure that's not a typo? Also, make sure to change "signal" to "socket" in the commit message, not just the PR title.

@Susurrus Susurrus force-pushed the constants branch 2 times, most recently from 93091ff to 24e7f6f Compare June 30, 2017 03:05
@Susurrus
Copy link
Contributor Author

@asomers Good catch on both of those, they've since been fixed. Now we just need our CI to work again!

@asomers
Copy link
Member

asomers commented Jun 30, 2017

Everything looks good to me. Too bad we can't tell bors yet.

@asomers
Copy link
Member

asomers commented Jul 2, 2017

bors r+

bors bot added a commit that referenced this pull request Jul 2, 2017
636: Rely on libc for more socket constants r=asomers

Not all values are upstreamed, but this covers the vast majority of them.
@bors
Copy link
Contributor

bors bot commented Jul 2, 2017

Build failed

@asomers
Copy link
Member

asomers commented Jul 2, 2017

Looks like the build failed due to undefined symbols in Linux for arm, i686, powerpc, musl, and netbsd. Also, the x86_64-unknown-linux test with Rust=beta failed due to PR #642

@Susurrus
Copy link
Contributor Author

Susurrus commented Jul 2, 2017

Yes, this is blocking on upstream (rust-lang/libc#639).

bors added a commit to rust-lang/libc that referenced this pull request Jul 3, 2017
Add socket constants for more platforms

Missing a few constants across all nix-supported platforms (see nix-rust/nix#636) so this adds them. This is still a work in progress as I wanted to make sure I didn't break anything doing most of the fixes. I'll come back and finish this up later.
@Susurrus
Copy link
Contributor Author

Susurrus commented Jul 3, 2017

bors r+

bors bot added a commit that referenced this pull request Jul 3, 2017
636: Rely on libc for more socket constants r=Susurrus

Not all values are upstreamed, but this covers the vast majority of them.
@bors
Copy link
Contributor

bors bot commented Jul 3, 2017

@bors bors bot merged commit c2d3422 into nix-rust:master Jul 3, 2017
@Susurrus Susurrus deleted the constants branch July 3, 2017 04:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants