-
Notifications
You must be signed in to change notification settings - Fork 13.3k
add methods to TCP and UDP sockets to modify hop limit #94678
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
Conversation
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
library/std/src/net/tcp.rs
Outdated
/// stream.set_multicast_hlim_v6(88).expect("set_hop_limit_v6 call failed"); | ||
/// ``` | ||
#[unstable(feature = "ipv6_hop_limit", issue = "47727")] | ||
pub fn set_multicast_hlim_v6(&self, limit: u32) -> io::Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why thi is "hlim" here but "hop_limit" for unicast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to keep the length of the method name from being too long. Should I change it to "hop_limit"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Rust doesn't generally abbreviate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the method names.
Since it adds new API surface: r? rust-lang/libs-api |
☔ The latest upstream changes (presumably #95565) made this pull request unmergeable. Please resolve the merge conflicts. |
This seems fine, assuming it passes CI. Let's see if the requisite constants are defined on all platforms... @bors r+ |
⌛ Testing commit b3e9e94 with merge f5cfbb23b1def2ab0d4dfe01c13340628b453cd6... |
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@bors r- Queue got messed up after a synchronize. |
☔ The latest upstream changes (presumably #98033) made this pull request unmergeable. Please resolve the merge conflicts. |
/// stream.set_hop_limit_v6(88).expect("set_hop_limit_v6 call failed"); | ||
/// ``` | ||
#[unstable(feature = "ipv6_hop_limit", issue = "47727")] | ||
pub fn set_hop_limit_v6(&self, limit: u32) -> io::Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hop Limit is an 8bit header field. So using u8
instead of u32
would reflect that better and avoid API misusage like #73158.
@kckeiks any updates on this? |
Sorry, I've been super busy, I will push an update sometime this week though. |
Just following up. My apologies, work has been keeping me exceedingly busy so I'm not sure when I'll have some bandwidth to resolve this. :/ |
That's fine, can understand. I am going to close this as it is inactive. If you find the time feel free to reöpen it or preferably create a new PR and we can take it from there. Thanks |
…=dtolnay Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678) rust-lang#94678 was closed because of time constraints of the original author (`@kckeiks).` This PR is a refresh of rust-lang#94678 where the original proposed changes are ported to the current status of the standard library. In addition, this PR: - implements the change suggested in rust-lang#94678 (comment) - defines `set_multicast_hop_limit_v6` and `multicast_hop_limit_v6 ` only for UDP sockets and not for TCP sockets as originally proposed Issue: rust-lang#139166
…=dtolnay Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678) rust-lang#94678 was closed because of time constraints of the original author (``@kckeiks).`` This PR is a refresh of rust-lang#94678 where the original proposed changes are ported to the current status of the standard library. In addition, this PR: - implements the change suggested in rust-lang#94678 (comment) - defines `set_multicast_hop_limit_v6` and `multicast_hop_limit_v6 ` only for UDP sockets and not for TCP sockets as originally proposed Issue: rust-lang#139166
…enton Rollup of 12 pull requests Successful merges: - rust-lang#138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678)) - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast) - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate) - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration) - rust-lang#139582 (Various coercion cleanups) - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`) - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`) - rust-lang#139666 (cleanup `mir_borrowck`) - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints) - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout) - rust-lang#139722 (Move some things to rustc_type_ir) - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test) r? `@ghost` `@rustbot` modify labels: rollup
This is my first addition of a new feature so let me know if I missed something in the process. I read the rustc dev guide about implementing new features but since the change here is simple, it suggested that a PR would be enough.
Fixes #47727