Skip to content

Conversation

de-vri-es
Copy link
Contributor

@de-vri-es de-vri-es commented Jun 16, 2021

This PR is a small maintainability improvement. It simplifies unix/net/ancillary.rs in std by removing the cfg_ifs for casting to the correct integer type, and just rely on libc to define the struct correctly.

@rustbot rustbot added A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` O-unix Operating system: Unix-like T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 16, 2021
@rust-highfive
Copy link
Contributor

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 16, 2021
@m-ou-se m-ou-se added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 17, 2021
if #[cfg(any(
target_os = "android",
all(target_os = "linux", target_env = "gnu"),
all(target_os = "linux", target_env = "uclibc"),
Copy link
Member

Choose a reason for hiding this comment

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

This cfg is different than the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some consideration, I decided to remove the cfg_if entirely and rely on libc to have the correct struct definition instead. If something turns out to be incorrect, it only needs to be fixed in one place :)

@m-ou-se m-ou-se assigned m-ou-se and unassigned dtolnay Jun 17, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jun 17, 2021

Nice! The fewer of these cfg blocks we have, the better. :)

@m-ou-se m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 17, 2021
@de-vri-es de-vri-es force-pushed the simplify-repeated-cfg-ifs branch 2 times, most recently from 9e09304 to d892738 Compare June 17, 2021 13:53
@rust-log-analyzer

This comment has been minimized.

@de-vri-es de-vri-es force-pushed the simplify-repeated-cfg-ifs branch from d892738 to 259bf5f Compare June 17, 2021 13:56
Comment on lines +309 to +312
let cmsg_len_zero = libc::CMSG_LEN(0) as usize;
let data_len = (*cmsg).cmsg_len as usize - cmsg_len_zero;
let data = libc::CMSG_DATA(cmsg).cast();
let data = from_raw_parts(data, data_len as usize);
let data = from_raw_parts(data, data_len);
Copy link
Contributor Author

@de-vri-es de-vri-es Jun 17, 2021

Choose a reason for hiding this comment

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

This can just always be a usize, because it's only ever passed to slice::from_raw_parts.

@m-ou-se
Copy link
Member

m-ou-se commented Jun 17, 2021

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 17, 2021

📌 Commit 259bf5f has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2021
@de-vri-es de-vri-es changed the title Factor out repeated cfg_if into type aliases in unix/net/ancillary.rs Rely on libc for correct integer types in os/unix/net/ancillary.rs. Jun 17, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 17, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#85925 (Linear interpolation)
 - rust-lang#86202 (Specialize `io::Bytes::size_hint` for more types)
 - rust-lang#86357 (Rely on libc for correct integer types in os/unix/net/ancillary.rs.)
 - rust-lang#86388 (Make `s` pre-interned)
 - rust-lang#86401 (Fix ICE when using `#[doc(keyword = "...")]` on non-items)
 - rust-lang#86405 (Add incr-comp note for 1.53.0 relnotes)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a5dce6c into rust-lang:master Jun 18, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-cleanup Category: PRs that clean code up or issues documenting cleanup. O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants