Skip to content

Commit

Permalink
Rollup merge of #136552 - ChrisDenton:option-find-handle, r=Mark-Simu…
Browse files Browse the repository at this point in the history
…lacrum

Use an `Option` for `FindNextFileHandle` in `ReadDir` instead of `INVALID_FILE_HANDLE` sentinel value

Sometimes we store an invalid handle when we don't want to return an error. We then check the handle before use in order to avoid actually using the invalid handle. However, using an `Option` for this is better and avoids us forgetting to check the handle is valid. This was noticed due to us closing the handle without checking for validity: https://github.com/rust-lang/rust/blob/bd6a6777f5cbbec549f123995026cef76d1e6b84/library/std/src/sys/pal/windows/fs.rs#L148-L151
  • Loading branch information
workingjubilee authored Feb 10, 2025
2 parents e1bd25e + bd6a677 commit f471ce3
Showing 1 changed file with 6 additions and 10 deletions.
16 changes: 6 additions & 10 deletions library/std/src/sys/pal/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct FileType {
}

pub struct ReadDir {
handle: FindNextFileHandle,
handle: Option<FindNextFileHandle>,
root: Arc<PathBuf>,
first: Option<c::WIN32_FIND_DATAW>,
}
Expand Down Expand Up @@ -113,13 +113,13 @@ impl fmt::Debug for ReadDir {
impl Iterator for ReadDir {
type Item = io::Result<DirEntry>;
fn next(&mut self) -> Option<io::Result<DirEntry>> {
if self.handle.0 == c::INVALID_HANDLE_VALUE {
let Some(handle) = self.handle.as_ref() else {
// This iterator was initialized with an `INVALID_HANDLE_VALUE` as its handle.
// Simply return `None` because this is only the case when `FindFirstFileExW` in
// the construction of this iterator returns `ERROR_FILE_NOT_FOUND` which means
// no matchhing files can be found.
return None;
}
};
if let Some(first) = self.first.take() {
if let Some(e) = DirEntry::new(&self.root, &first) {
return Some(Ok(e));
Expand All @@ -128,7 +128,7 @@ impl Iterator for ReadDir {
unsafe {
let mut wfd = mem::zeroed();
loop {
if c::FindNextFileW(self.handle.0, &mut wfd) == 0 {
if c::FindNextFileW(handle.0, &mut wfd) == 0 {
match api::get_last_error() {
WinError::NO_MORE_FILES => return None,
WinError { code } => {
Expand Down Expand Up @@ -1194,7 +1194,7 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {

if find_handle != c::INVALID_HANDLE_VALUE {
Ok(ReadDir {
handle: FindNextFileHandle(find_handle),
handle: Some(FindNextFileHandle(find_handle)),
root: Arc::new(root),
first: Some(wfd),
})
Expand All @@ -1212,11 +1212,7 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
// See issue #120040: https://github.com/rust-lang/rust/issues/120040.
let last_error = api::get_last_error();
if last_error == WinError::FILE_NOT_FOUND {
return Ok(ReadDir {
handle: FindNextFileHandle(find_handle),
root: Arc::new(root),
first: None,
});
return Ok(ReadDir { handle: None, root: Arc::new(root), first: None });
}

// Just return the error constructed from the raw OS error if the above is not the case.
Expand Down

0 comments on commit f471ce3

Please sign in to comment.