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

Elaborate on ip_addr bit conversion endianness #118505

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

CLEckhardt
Copy link
Contributor

Adds explanation of how endianness is handled when converting Ipv4Addr and Ipv6Addr to and from bits. This is intended to unblock stabilization of the affected methods.

Addresses #113744

@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cuviper (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 1, 2023
/// Although IPv4 addresses are big-endian, the `u32` value will use the target platform's
/// native byte order. That is, the `u32` value is an integer representation of the IPv4
/// address and not an integer interpretation of the IPv4 address's big-endian bitstring. This
/// means that the `u32` value `&` with `0xffffff00` will set the last octet in the address to
Copy link
Member

Choose a reason for hiding this comment

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

Using the & operator as a verb seems confusing to me -- maybe we can leave that to the example and use "masked" here?

Suggested change
/// means that the `u32` value `&` with `0xffffff00` will set the last octet in the address to
/// means that the `u32` value masked with `0xffffff00` will set the last octet in the address to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds a lot better, thanks. Update pushed.

/// Although IPv6 addresses are big-endian, the `u128` value will use the target platform's
/// native byte order. That is, the `u128` value is an integer representation of the IPv6
/// address and not an integer interpretation of the IPv6 address's big-endian bitstring. This
/// means that the `u128` value `&` with `0xffffffffffffffffffffffffffff0000_u128` will set the
Copy link
Member

Choose a reason for hiding this comment

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

Same confusion and suggestion as above.

Suggested change
/// means that the `u128` value `&` with `0xffffffffffffffffffffffffffff0000_u128` will set the
/// means that the `u128` value masked with `0xffffffffffffffffffffffffffff0000_u128` will set the

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update pushed.

Adds explanation of how endianness is handled when converting `Ipv4Addr`
and `Ipv6Addr` to and from bits.

Addresses rust-lang#113744
@CLEckhardt CLEckhardt force-pushed the update_ip_addr_bits_docs branch from 73382cb to c3bb1b5 Compare December 7, 2023 15:16
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=CLEckhardt
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_35072ea5-012b-4369-b8e5-f478bc163272
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=update_ip_addr_bits_docs
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_35072ea5-012b-4369-b8e5-f478bc163272
GITHUB_REF=refs/pull/118505/merge
GITHUB_REF_NAME=118505/merge
GITHUB_REF_PROTECTED=false
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=e1d61fd2d037a6d8a7937f7d4c141a2c7007e558
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_35072ea5-012b-4369-b8e5-f478bc163272
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_35072ea5-012b-4369-b8e5-f478bc163272
GITHUB_TRIGGERING_ACTOR=CLEckhardt
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/118505/merge
GITHUB_WORKFLOW_SHA=e1d61fd2d037a6d8a7937f7d4c141a2c7007e558
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Built container sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Looks like docker image is the same as before, not uploading
https://ci-caches.rust-lang.org/docker/7ebc15c01a233894034d277c8cce4e949f4e7791f66b4727c8fb6e058a0b8171d6152e1441d677cef0653843ceeee469c097b8699b2bb74249e674f6aa1a8813
sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Thu Dec  7 15:18:44 UTC 2023
  network time: Thu, 07 Dec 2023 15:18:45 GMT
  network time: Thu, 07 Dec 2023 15:18:45 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-missing-tools', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: rust.codegen-units-std := 1
---
   Compiling bitflags v2.4.0
   Compiling linux-raw-sys v0.4.5
   Compiling fastrand v2.0.0
   Compiling smallvec v1.10.0
error: invalid `--check-cfg` argument: `values(freebsd10)` (expected `cfg(name, values("value1", "value2", ... "valueN"))`)
error: could not compile `libc` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:10:37
  local time: Thu Dec  7 15:29:56 UTC 2023

@CLEckhardt
Copy link
Contributor Author

@rustbot review

@cuviper
Copy link
Member

cuviper commented Dec 8, 2023

Looks good!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 8, 2023

📌 Commit c3bb1b5 has been approved by cuviper

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 8, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#118505 (Elaborate on ip_addr bit conversion endianness)
 - rust-lang#118581 (OnceLock: Add note about drop and statics)
 - rust-lang#118677 ([rustdoc] Fix display of features)
 - rust-lang#118690 (coverage: Avoid unnecessary macros in unit tests)
 - rust-lang#118693 (Tell MirUsedCollector that the pointer alignment checks calls its panic symbol)
 - rust-lang#118695 (coverage: Merge refined spans in a separate final pass)
 - rust-lang#118709 (fix jobserver GLOBAL_CLIENT_CHECKED uninitialized before use)
 - rust-lang#118722 (rustdoc: remove unused parameter `reversed` from onEach(Lazy))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 992f7ee into rust-lang:master Dec 8, 2023
8 of 11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 8, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2023
Rollup merge of rust-lang#118505 - CLEckhardt:update_ip_addr_bits_docs, r=cuviper

Elaborate on ip_addr bit conversion endianness

Adds explanation of how endianness is handled when converting `Ipv4Addr` and `Ipv6Addr` to and from bits. This is intended to unblock stabilization of the affected methods.

Addresses rust-lang#113744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants