Skip to content

Update ReadDir::next in std::sys::pal::unix::fs to use &raw const (*p).field instead of p.byte_offset().cast()#134678

Merged
bors merged 1 commit into
rust-lang:masterfrom
zachs18:offset-ptr-update
Jan 15, 2025
Merged

Update ReadDir::next in std::sys::pal::unix::fs to use &raw const (*p).field instead of p.byte_offset().cast()#134678
bors merged 1 commit into
rust-lang:masterfrom
zachs18:offset-ptr-update

Conversation

@zachs18
Copy link
Copy Markdown
Contributor

@zachs18 zachs18 commented Dec 23, 2024

Since rust-lang/reference#1387 and #117572, &raw mut (*p).field/addr_of!((*p).field) is defined to have the same inbounds preconditions as ptr::offset/ptr::byte_offset. I.e. &raw const (*p).field does not require that p: *const T point to a full size_of::<T>() bytes of memory, only that p.byte_add(offset_of!(T, field)) is defined.

The old comment "[...] we don't even get to use &raw const (*entry_ptr).d_name because that operation requires the full extent of *entry_ptr to be in bounds of the same allocation, which is not necessarily the case here [...]" is now outdated, and the code can be simplified to use &raw const (*entry_ptr).field.


There should be no behavior differences from this PR.

The : *const dirent64 on line 716 and the const _: usize = mem::offset_of!(dirent64, $field); and comment on lines 749-751 are just sanity checks and should not affect semantics.

Since the offset_ptr! macro is only called three times, and all with the same local variable entry_ptr, I just used the local variable directly in the macro instead of taking it as an input, and renamed the macro to entry_field_ptr!.

The whole macro could also be removed and replaced with just using &raw const (*entry_ptr).field in the three places, but the comments on the macro seemed worthwhile to keep.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

5 participants