use offset_of! to calculate dirent64 field offsets #114519
Conversation
the8472
added
the
T-libs
rustbot
added
the
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
| .offset_from(entry_ptr.cast::<u8>()) | ||
| } | ||
| }; | ||
| const OFFSET: isize = mem::offset_of!(dirent64, $field) as isize; | ||
| if true { | ||
| // Cast to the same type determined by the else branch. | ||
| $entry_ptr.byte_offset(OFFSET).cast::<_>() |
There was a problem hiding this comment.
curiosity: I see above it talks about "in bounds of the same allocation, which is not necessarily the case here", but then it uses byte_offset, which also has the "inside the same allocation" problem.
Should it perhaps be wrapping_byte_offset here? Or am I misunderstanding the restrictions under which this is operating?
There was a problem hiding this comment.
I think both the code and comment are correct as written. byte_offset(OFFSET) requires the start of $field to be in bounds, which it is. addr_of!((*ptr).$field) requires the start and end of $field (and every other field) to be in bounds which they are not.
There was a problem hiding this comment.
Ah, thanks! I hadn't considered the whole width of the field problem.
|
⌛ Testing commit 20c25d6 with merge c88b7fdfd99eda0e12376d12a4aa271d66e3033b... |
|
☀️ Test successful - checks-actions |
|
👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward |
|
Finished benchmarking commit (c88b7fdfd99eda0e12376d12a4aa271d66e3033b): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 652.053s -> 652.033s (-0.00%) |
|
weird @bors retry |
|
@bors r=dtolnay |
|
💡 This pull request was already approved, no need to approve it again.
|
|
and the same thing with #113648, suspiciously often. |
| // need to make our own non-libc allocation that conforms to the weird | ||
| // imaginary definition of dirent64, and use that for a field offset | ||
| // computation. | ||
| // Instead we must access fields individually through their offsets. | ||
| macro_rules! offset_ptr { |
There was a problem hiding this comment.
FWIW, if rust-lang/reference#1387 gets accepted then all this can be removed and a direct addr_of!((*entry_ptr).d_name) should work.
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#114466 (Add Allocation to SMIR) - rust-lang#114505 (Add documentation to has_deref) - rust-lang#114519 (use offset_of! to calculate dirent64 field offsets) - rust-lang#114537 (Migrate GUI colors test to original CSS color format) - rust-lang#114539 (linkchecker: Remove unneeded FIXME about intra-doc links) Failed merges: - rust-lang#114485 (Add trait decls to SMIR) r? `@ghost` `@rustbot` modify labels: rollup
r? @dtolnay