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

std: Add a net module for TCP/UDP #22015

Merged
merged 1 commit into from
Feb 12, 2015
Merged

std: Add a net module for TCP/UDP #22015

merged 1 commit into from
Feb 12, 2015

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 807 which adds a std::net
module for basic neworking based on top of std::io. This module serves as a
replacement for the std::old_io::net module and networking primitives in
old_io.

The major focus of this redesign is to cut back on the level of abstraction to
the point that each of the networking types is just a bare socket. To this end
functionality such as timeouts and cloning has been removed (although cloning
can be done through duplicate, it may just yield an error).

With this net module comes a new implementation of SocketAddr and IpAddr.
This work is entirely based on #20785 and the only changes were to alter the
in-memory representation to match the libc-expected variants and to move from
public fields to accessors.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

Note that this is an in progress implementation of rust-lang/rfcs#807 and should not be merged until the RFC is merge. All future updates to the RFC will be merged in here as well.

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned brson Feb 6, 2015
@aturon
Copy link
Member

aturon commented Feb 6, 2015

cc @sfackler @carllerche


// ////////////////////////////////////////////////////////////////////////////////
// // get_address_name
// ////////////////////////////////////////////////////////////////////////////////
Copy link
Member

Choose a reason for hiding this comment

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

Is this intentionally left here? Should it be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, meant to delete this.

@carllerche
Copy link
Member

👍 looks good to me. As I discussed a/ @alexcrichton, I believe that the only blocker for me to remove my re-impl of the net types with this is being able to construct the various types (listener, stream, ...) from an FD. The reason that I need this is, as mentioned in the RFC, more advanced pre-configuration of the socket types is being deferred. I (and other mio users) really need to be able to do more than what is exposed. An escape valve is being able to construct the types from an FD and get the FD from the types (getting is already possible). All that would be left is allowing construction of the various types from the FD.

@alexcrichton alexcrichton force-pushed the netv2 branch 3 times, most recently from a51a03d to 3e6b4af Compare February 6, 2015 20:43
SocketAddr { repr: Repr::V4(addr) }
}
}
impl FromInner<libc::sockaddr_in6> for SocketAddr {
Copy link
Member

Choose a reason for hiding this comment

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

Needs a line above.

@aturon aturon mentioned this pull request Feb 6, 2015
38 tasks
@alexcrichton alexcrichton force-pushed the netv2 branch 2 times, most recently from e284394 to e030074 Compare February 10, 2015 07:16
/// A trait for objects which can be converted or resolved to one or more
/// `SocketAddr` values.
///
/// This trait is used for generic address resolution when constructing network
Copy link
Member

Choose a reason for hiding this comment

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

This is so nice <3

@aturon
Copy link
Member

aturon commented Feb 11, 2015

I'm basically happy with this PR. I left a couple of minor nits, mostly bikeshedding.

@aturon
Copy link
Member

aturon commented Feb 11, 2015

@carllerche Yes, we need to tackle raw creation soon, though that will be needed across all of new IO. Should be easy to do.

This commit is an implementation of [RFC 807][rfc] which adds a `std::net`
module for basic neworking based on top of `std::io`. This module serves as a
replacement for the `std::old_io::net` module and networking primitives in
`old_io`.

[rfc]: fillmein

The major focus of this redesign is to cut back on the level of abstraction to
the point that each of the networking types is just a bare socket. To this end
functionality such as timeouts and cloning has been removed (although cloning
can be done through `duplicate`, it may just yield an error).

With this `net` module comes a new implementation of `SocketAddr` and `IpAddr`.
This work is entirely based on rust-lang#20785 and the only changes were to alter the
in-memory representation to match the `libc`-expected variants and to move from
public fields to accessors.
@alexcrichton
Copy link
Member Author

Updated with nits addressed.

@alexcrichton
Copy link
Member Author

@bors: r=aturon 395709c

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 11, 2015
This commit is an implementation of [RFC 807][rfc] which adds a `std::net`
module for basic neworking based on top of `std::io`. This module serves as a
replacement for the `std::old_io::net` module and networking primitives in
`old_io`.

[rfc]: fillmein

The major focus of this redesign is to cut back on the level of abstraction to
the point that each of the networking types is just a bare socket. To this end
functionality such as timeouts and cloning has been removed (although cloning
can be done through `duplicate`, it may just yield an error).

With this `net` module comes a new implementation of `SocketAddr` and `IpAddr`.
This work is entirely based on rust-lang#20785 and the only changes were to alter the
in-memory representation to match the `libc`-expected variants and to move from
public fields to accessors.
@bors
Copy link
Contributor

bors commented Feb 12, 2015

⌛ Testing commit 395709c with merge 312627b...

@bors
Copy link
Contributor

bors commented Feb 12, 2015

💔 Test failed - auto-linux-64-x-android-t

@alexcrichton
Copy link
Member Author

@bors: retry

@bors bors merged commit 395709c into rust-lang:master Feb 12, 2015
@alexcrichton alexcrichton deleted the netv2 branch February 12, 2015 02:54
@aturon aturon mentioned this pull request Feb 18, 2015
91 tasks
Ms2ger added a commit to servo/rfcs that referenced this pull request Mar 25, 2015
The alleged implementation of this RFC in
<rust-lang/rust#22015> included `try_clone` rather
than `duplicate`. This commit updates the approved RFC to match the actual
implementation.
Centril added a commit to Centril/rust that referenced this pull request Dec 16, 2019
Delete flaky test net::tcp::tests::fast_rebind

This test is unreliable for at least 3 users on two platforms: see rust-lang#57509 and rust-lang#51006. It was added 5 years ago in rust-lang#22015. Do we know whether this is testing something important that would indicate a bug in our implementation, or if it's fine to remove?

r? @sfackler @alexcrichton because this somewhat resembles rust-lang#59018

Closes rust-lang#57509. Closes rust-lang#51006.
Centril added a commit to Centril/rust that referenced this pull request Dec 16, 2019
Delete flaky test net::tcp::tests::fast_rebind

This test is unreliable for at least 3 users on two platforms: see rust-lang#57509 and rust-lang#51006. It was added 5 years ago in rust-lang#22015. Do we know whether this is testing something important that would indicate a bug in our implementation, or if it's fine to remove?

r? @sfackler @alexcrichton because this somewhat resembles rust-lang#59018

Closes rust-lang#57509. Closes rust-lang#51006.
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.

9 participants