Skip to content

Commit 0bb6eb1

Browse files
committed
Eliminate 280-byte memset from ReadDir iterator
1 parent a501e66 commit 0bb6eb1

File tree

1 file changed

+65
-20
lines changed
  • library/std/src/sys/unix

1 file changed

+65
-20
lines changed

Diff for: library/std/src/sys/unix/fs.rs

+65-20
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ use crate::ffi::{CStr, OsStr, OsString};
44
use crate::fmt;
55
use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
66
use crate::mem;
7+
#[cfg(any(
8+
target_os = "android",
9+
target_os = "linux",
10+
target_os = "solaris",
11+
target_os = "fuchsia",
12+
target_os = "redox",
13+
target_os = "illumos"
14+
))]
15+
use crate::mem::MaybeUninit;
716
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
817
use crate::path::{Path, PathBuf};
918
use crate::ptr;
@@ -584,33 +593,69 @@ impl Iterator for ReadDir {
584593
};
585594
}
586595

587-
// Only d_reclen bytes of *entry_ptr are valid, so we can't just copy the
588-
// whole thing (#93384). Instead, copy everything except the name.
589-
let mut copy: dirent64 = mem::zeroed();
590-
// Can't dereference entry_ptr, so use the local entry to get
591-
// offsetof(struct dirent, d_name)
592-
let copy_bytes = &mut copy as *mut _ as *mut u8;
593-
let copy_name = &mut copy.d_name as *mut _ as *mut u8;
594-
let name_offset = copy_name.offset_from(copy_bytes) as usize;
595-
let entry_bytes = entry_ptr as *const u8;
596-
let entry_name = entry_bytes.add(name_offset);
597-
ptr::copy_nonoverlapping(entry_bytes, copy_bytes, name_offset);
596+
// The dirent64 struct is a weird imaginary thing that isn't ever supposed
597+
// to be worked with by value. Its trailing d_name field is declared
598+
// variously as [c_char; 256] or [c_char; 1] on different systems but
599+
// either way that size is meaningless; only the offset of d_name is
600+
// meaningful. The dirent64 pointers that libc returns from readdir64 are
601+
// allowed to point to allocations smaller _or_ LARGER than implied by the
602+
// definition of the struct.
603+
//
604+
// As such, we need to be even more careful with dirent64 than if its
605+
// contents were "simply" partially initialized data.
606+
//
607+
// Like for uninitialized contents, converting entry_ptr to `&dirent64`
608+
// would not be legal. However, unique to dirent64 is that we don't even
609+
// get to use `addr_of!((*entry_ptr).d_name)` because that operation
610+
// requires the full extent of *entry_ptr to be in bounds of the same
611+
// allocation, which is not necessarily the case here.
612+
//
613+
// Absent any other way to obtain a pointer to `(*entry_ptr).d_name`
614+
// legally in Rust analogously to how it would be done in C, we instead
615+
// need to make our own non-libc allocation that conforms to the weird
616+
// imaginary definition of dirent64, and use that for a field offset
617+
// computation.
618+
macro_rules! offset_ptr {
619+
($entry_ptr:expr, $field:ident) => {{
620+
const OFFSET: isize = {
621+
let delusion = MaybeUninit::<dirent64>::uninit();
622+
let entry_ptr = delusion.as_ptr();
623+
unsafe {
624+
ptr::addr_of!((*entry_ptr).$field)
625+
.cast::<u8>()
626+
.offset_from(entry_ptr.cast::<u8>())
627+
}
628+
};
629+
if true {
630+
// Cast to the same type determined by the else branch.
631+
$entry_ptr.byte_offset(OFFSET).cast::<_>()
632+
} else {
633+
#[allow(deref_nullptr)]
634+
{
635+
ptr::addr_of!((*ptr::null::<dirent64>()).$field)
636+
}
637+
}
638+
}};
639+
}
640+
641+
// d_name is guaranteed to be null-terminated.
642+
let name = CStr::from_ptr(offset_ptr!(entry_ptr, d_name).cast());
643+
let name_bytes = name.to_bytes();
644+
if name_bytes == b"." || name_bytes == b".." {
645+
continue;
646+
}
598647

599648
let entry = dirent64_min {
600-
d_ino: copy.d_ino as u64,
649+
d_ino: *offset_ptr!(entry_ptr, d_ino) as u64,
601650
#[cfg(not(any(target_os = "solaris", target_os = "illumos")))]
602-
d_type: copy.d_type as u8,
651+
d_type: *offset_ptr!(entry_ptr, d_type) as u8,
603652
};
604653

605-
let ret = DirEntry {
654+
return Some(Ok(DirEntry {
606655
entry,
607-
// d_name is guaranteed to be null-terminated.
608-
name: CStr::from_ptr(entry_name as *const _).to_owned(),
656+
name: name.to_owned(),
609657
dir: Arc::clone(&self.inner),
610-
};
611-
if ret.name_bytes() != b"." && ret.name_bytes() != b".." {
612-
return Some(Ok(ret));
613-
}
658+
}));
614659
}
615660
}
616661
}

0 commit comments

Comments
 (0)