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

socket: Respect IPv6 flowinfo and scope_id in InetAddr::from_std #335

Merged
merged 1 commit into from
Mar 31, 2016

Conversation

kamalmarhubi
Copy link
Member

Fixes #329

sin6_port: addr.port().to_be(), // network byte order
sin6_addr: Ipv6Addr::from_std(addr.ip()).0,
sin6_flowinfo: addr.flowinfo().to_be(), // network byte order
sin6_scope_id: addr.scope_id().to_be(), // network byte order
Copy link

Choose a reason for hiding this comment

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

sin6_flowinfo and sin6_scope_id must be in host byte order.
Related: rust-lang/rust#32424

Copy link
Member Author

Choose a reason for hiding this comment

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

But the port should be in network byte order, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

From rust-lang/rust#32429 looks like only the port. Should be fixed now!

@misuzu
Copy link

misuzu commented Mar 30, 2016

Looks great! Can't really test it as build is failing on nightly (nightly is required because of rust-lang/rust#32424).

@kamalmarhubi
Copy link
Member Author

@misuzu it builds locally at 8422bfa on my machine. I think the CI is more strict with raw_pointer_derive. If you have time, could you check out that commit and see if it works?

@kamalmarhubi
Copy link
Member Author

Actually, if you have a test case we can add, that would be even better!

@misuzu
Copy link

misuzu commented Mar 30, 2016

@kamalmarhubi std::net:SocketAddrV6 is broken on stable so test also will be broken. That stuff fixed in nightly 2016-03-25+
Seems like there is some bug in rustc.

$ rustc -V
rustc 1.9.0-nightly (b678600ac 2016-03-29)
$ cargo build
   Compiling nix v0.5.1-pre (file:///home/misuzu/Documents/Rust/nix-rust)
src/sys/signal.rs:157:9: 157:27 warning: lint raw_pointer_derive has been removed: using derive with raw pointers is ok, #[warn(renamed_and_removed_lints)] on by default
src/sys/signal.rs:157 #[allow(raw_pointer_derive)]
                              ^~~~~~~~~~~~~~~~~~
src/sys/signal.rs:157:9: 157:27 warning: lint raw_pointer_derive has been removed: using derive with raw pointers is ok, #[warn(renamed_and_removed_lints)] on by default
src/sys/signal.rs:157 #[allow(raw_pointer_derive)]
                              ^~~~~~~~~~~~~~~~~~
src/sys/signal.rs:158:23: 158:32 note: in this expansion of #[derive_PartialEq] (defined in src/sys/signal.rs)
src/sys/signal.rs:157:9: 157:27 warning: lint raw_pointer_derive has been removed: using derive with raw pointers is ok, #[warn(renamed_and_removed_lints)] on by default
src/sys/signal.rs:157 #[allow(raw_pointer_derive)]
                              ^~~~~~~~~~~~~~~~~~
src/sys/signal.rs:158:17: 158:21 note: in this expansion of #[derive_Copy] (defined in src/sys/signal.rs)
src/sys/signal.rs:157:9: 157:27 warning: lint raw_pointer_derive has been removed: using derive with raw pointers is ok, #[warn(renamed_and_removed_lints)] on by default
src/sys/signal.rs:157 #[allow(raw_pointer_derive)]
                              ^~~~~~~~~~~~~~~~~~
src/sys/signal.rs:158:10: 158:15 note: in this expansion of #[derive_Clone] (defined in src/sys/signal.rs)
src/sched.rs:208:20: 208:34 warning: `extern "C" fn(*mut Box<core::ops::FnMut() -> isize>) -> i32 {sched::clone::callback}` is now zero-sized and has to be cast to a pointer before transmuting to `*const extern "C" fn(*const Box<core::ops::FnMut() -> isize>) -> i32`, #[warn(transmute_from_fn_item_types)] on by default
src/sched.rs:208         ffi::clone(mem::transmute(callback), ptr as *mut c_void, flags.bits(), &mut cb)
                                    ^~~~~~~~~~~~~~
src/sched.rs:208:20: 208:34 warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
src/sched.rs:208:20: 208:34 note: for more information, see issue #19925 <https://github.com/rust-lang/rust/issues/19925>
rustc: /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/llvm/include/llvm/Support/Casting.h:237: typename llvm::cast_retty<X, Y*>::ret_type llvm::cast(Y*) [with X = llvm::Function; Y = llvm::Value; typename llvm::cast_retty<X, Y*>::ret_type = llvm::Function*]: Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
error: Could not compile `nix`.

To learn more, run the command again with --verbose.

@misuzu
Copy link

misuzu commented Mar 30, 2016

I was wrong, this fix doesn't need rust-lang/rust#32424
You can add this test to test/sys/test_socket.rs

#[test]
pub fn test_inetv6_addr_to_sock_addr() {
    let port: u16 = 3000;
    let flowinfo: u32 = 1;
    let scope_id: u32 = 2;
    let actual = net::SocketAddr::V6(net::SocketAddrV6::new("fe80::1".parse().unwrap(), port, flowinfo, scope_id));
    let addr = InetAddr::from_std(&actual);

    match addr {
        InetAddr::V6(addr) => {
            assert_eq!(addr.sin6_port, port.to_be());
            assert_eq!(addr.sin6_flowinfo, flowinfo);
            assert_eq!(addr.sin6_scope_id, scope_id);
        }
        _ => panic!("nope"),
    }

    assert_eq!(actual, addr.to_std());
}

@kamalmarhubi
Copy link
Member Author

Added the test, thanks!

@kamalmarhubi
Copy link
Member Author

r? @fiveop

@fiveop
Copy link
Contributor

fiveop commented Mar 31, 2016

@homu r+

@homu
Copy link
Contributor

homu commented Mar 31, 2016

📌 Commit 6d01a26 has been approved by fiveop

@homu
Copy link
Contributor

homu commented Mar 31, 2016

⚡ Test exempted - status

@homu homu merged commit 6d01a26 into nix-rust:master Mar 31, 2016
homu added a commit that referenced this pull request Mar 31, 2016
socket: Respect IPv6 flowinfo and scope_id in InetAddr::from_std

Fixes #329
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.

None yet

4 participants