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::io::net may need more thought #13537

Closed
2 of 7 tasks
alexcrichton opened this issue Apr 15, 2014 · 6 comments
Closed
2 of 7 tasks

std::io::net may need more thought #13537

alexcrichton opened this issue Apr 15, 2014 · 6 comments
Labels
P-medium Medium priority

Comments

@alexcrichton
Copy link
Member

The net module has a few things which I would like to consider improving:

  • Must the Acceptor and Listener traits exists? It's a shame having to import the traits just to make a simple server.
  • Does the layout make sense? std::io::net::ip is quite long. Possibly a top-level net module? Possibly shorten using reexports?
  • What more needs to be exported? The current primitives implement the basic Reader/Writer traits, but not much beyond that. There are many methods provided by librustuv/libnative which are not exported. Need to make sure the signatures are correct.
  • "Unix pipes" are not unix pipes on windows (they are named pipes)

Wish list

  • TcpStream::open("google.com:80") does not work
  • I can clone a tcp stream, but I cannot close that duplicate tcp stream (from my owned stream)
  • Creating a server is quite wordy. There are a number of imports, lots of ip addr configuration, lots of listening/accepting, etc.

Nominating, I believe it is quite important to have a solid networking story.

@sinistersnare
Copy link
Contributor

Does net need to be in std?

I guess this is me agreeing that there should be a top level net module...

@liigo
Copy link
Contributor

liigo commented Apr 15, 2014

And std::io::fs too long too, std::fs? libfs? After std facade…

@sfackler
Copy link
Member

TcpStream::open("google.com:80") would be pretty straghtforward to implement, but we'd need to pull liburl into libstd most likely.

@alexcrichton
Copy link
Member Author

I believe that with the "libstd facade" that net does belong in std, but it perhaps belongs in its own crate behind std (such as a libio and a libnet), but these are just mild musings of me.

I do think that a "libstd facade" will make implementing TcpStream::open("google.com:80") nicer to stomach.

@pnkfelix
Copy link
Member

Marking P-high, not 1.0 milestone. Specific bugs can/should be opened if there are particular items that you want to assign higher priority (i.e. want to get them on the 1.0 milestone).

bors added a commit that referenced this issue May 11, 2014
…en, r=alexcrichton

Been meaning to try my hand at something like this for a while, and noticed something similar mentioned as part of #13537. The suggestion on the original ticket is to use `TcpStream::open(&str)` to pass in a host + port string, but seems a little cleaner to pass in host and port separately -- so a signature like `TcpStream::open(&str, u16)`.

Also means we can use std::io::net::addrinfo directly instead of using e.g. liburl to parse the host+port pair from a string.

One outstanding issue in this PR that I'm not entirely sure how to address: in open_timeout, the timeout_ms will apply for every A record we find associated with a hostname -- probably not the intended behavior, but I didn't want to waste my time on elaborate alternatives until the general idea was a-OKed. :)

Anyway, perhaps there are other reasons for us to prefer the original proposed syntax, but thought I'd get some thoughts on this. Maybe there are some solid reasons to prefer using liburl to do this stuff.
bors added a commit that referenced this issue May 13, 2014
…en, r=alexcrichton

Been meaning to try my hand at something like this for a while, and noticed something similar mentioned as part of #13537. The suggestion on the original ticket is to use `TcpStream::open(&str)` to pass in a host + port string, but seems a little cleaner to pass in host and port separately -- so a signature like `TcpStream::open(&str, u16)`.

Also means we can use std::io::net::addrinfo directly instead of using e.g. liburl to parse the host+port pair from a string.

One outstanding issue in this PR that I'm not entirely sure how to address: in open_timeout, the timeout_ms will apply for every A record we find associated with a hostname -- probably not the intended behavior, but I didn't want to waste my time on elaborate alternatives until the general idea was a-OKed. :)

Anyway, perhaps there are other reasons for us to prefer the original proposed syntax, but thought I'd get some thoughts on this. Maybe there are some solid reasons to prefer using liburl to do this stuff.
@brson
Copy link
Contributor

brson commented Jan 13, 2015

Seems like this issue has bitrotted.

@brson brson closed this as completed Jan 13, 2015
notriddle pushed a commit to notriddle/rust that referenced this issue Nov 10, 2022
minor: Allow ovsx publishing to fail
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 18, 2024
Mark unnecessary_first_then_check and byte_char_slices as Applicable

I don't really see situations where this isn't Applicable that aren't weird edge cases where the lint should be disabled.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants