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: Add get/set reuseport and get_localaddr/reuseaddr for TcpSocket #3083

Merged
merged 4 commits into from
Nov 2, 2020

Conversation

zekisherif
Copy link
Contributor

Motivation

Recently updated mio tcp sockets with getter/setter for SO_REUSEPORT and also added TcpSocket::get_localaddr and TcpSocket::get_reuseaddr.

This PR is just to make it available for tokio TcpSocket

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-net Module: tokio/net labels Nov 1, 2020
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Overall, this seems good. However, I think we should change the naming scheme to match the rest of Tokio. Typically, idiomatic Rust doesn't prefix accessor functions with get_. For example, a similar property on TcpListener has a setter called set_nodelay and a getter that's just called nodelay:

pub fn set_nodelay(&self, nodelay: bool) -> io::Result<()> {
self.io.get_ref().set_nodelay(nodelay)
}

pub fn nodelay(&self) -> io::Result<bool> {
self.io.get_ref().nodelay()
}

Let's follow that style here for consistency with the rest of Tokio's API.

tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
tokio/src/net/tcp/socket.rs Outdated Show resolved Hide resolved
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this looks good to me!

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 C-enhancement Category: A PR with an enhancement or bugfix. M-net Module: tokio/net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants