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

net: improve from_std docs #5332

Merged
merged 5 commits into from
Jan 11, 2023
Merged

Conversation

satakuma
Copy link
Member

@satakuma satakuma commented Jan 3, 2023

Motivation

Unlike documentation for other methods performing conversion from std types, docs for the TcpSocket::from_std_stream method do not warn about setting the socket in non-blocking mode.
The attached example also doesn't set the socket in non-blocking mode.

Solution

This change adds a suitable comment in the docs and sets the socket in non-blocking mode in the example.

@@ -666,6 +666,7 @@ impl TcpSocket {
/// socket must not have been connected prior to calling this function. This
/// function is typically used together with crates such as [`socket2`] to
/// configure socket options that are not available on `TcpSocket`.
/// It is left up to the user to set the socket in non-blocking mode.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this ought to be explicitly marked as a Note or Warning so that it's very obvious to users? Perhaps the warning also should note what will happen if users don't do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree.

Copy link
Member Author

Choose a reason for hiding this comment

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

I applied your suggestion and created a more obvious section about this. I did the same with other docs containing the same warning and added missing examples.

@Darksonn Darksonn added T-docs Topic: documentation A-tokio Area: The main tokio crate M-net Module: tokio/net labels Jan 10, 2023
@satakuma satakuma changed the title net: improve TcpSocket::from_std_stream docs net: improve from_std docs Jan 10, 2023
///
/// #[tokio::main]
/// async fn main() -> Result<(), Box<dyn Error>> {
/// let tokio_socket = tokio::net::UnixDatagram::bind("127.0.0.1:0")?;
Copy link
Member Author

Choose a reason for hiding this comment

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

Some examples used IP addresses to bind Unix sockets.

///
/// The caller is responsible for ensuring that the listener is in
/// non-blocking mode. Otherwise all I/O operations on the listener
/// will block the thread, what can cause unexpected behavior.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// will block the thread, what can cause unexpected behavior.
/// will block the thread, which will cause unexpected behavior.

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Darksonn Darksonn enabled auto-merge (squash) January 11, 2023 12:48
@Darksonn Darksonn merged commit f9dbfa8 into tokio-rs:master Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-net Module: tokio/net T-docs Topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants