From 567998c159a66bc8c2c3104e65b105b92234d869 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 20 Mar 2025 22:51:43 +0000 Subject: [PATCH] Fix are_file_paths_equivalent on Windows Trying to get an exclusive lock on the files was too flaky and also prone to false positives. --- Cargo.lock | 1 + Cargo.toml | 3 +++ src/archive/find.rs | 48 ++++++++++++++++++++++++--------------------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3ca6a175..3c7e598d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -451,6 +451,7 @@ dependencies = [ "saphyr-parser", "tempfile", "unicase", + "windows", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index a47617d1..24ab2fcf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,9 @@ saphyr-parser = "0.0.3" unicase = "2.8.1" rayon = "1.10.0" +[target.'cfg(windows)'.dependencies] +windows = { version = "0.60.0", features = ["Win32_Storage_FileSystem"] } + [dev-dependencies] rstest = "0.25.0" rstest_reuse = "0.7.0" diff --git a/src/archive/find.rs b/src/archive/find.rs index 7896224e..ae9fb1ac 100644 --- a/src/archive/find.rs +++ b/src/archive/find.rs @@ -1,7 +1,4 @@ -use std::{ - fs::OpenOptions, - path::{Path, PathBuf}, -}; +use std::path::{Path, PathBuf}; use crate::{GameType, game::GameCache, plugin::has_ascii_extension}; @@ -136,32 +133,39 @@ fn are_file_paths_equivalent(lhs: &Path, rhs: &Path) -> bool { return true; } - // See - // Or windows::Win32::Foundation::ERROR_SHARING_VIOLATION in the "windows" crate. - const ERROR_SHARING_VIOLATION: i32 = 32; - use std::os::windows::fs::OpenOptionsExt; + use std::fs::File; + use std::os::windows::io::AsRawHandle; + use windows::Win32::{ + Foundation::HANDLE, + Storage::FileSystem::{BY_HANDLE_FILE_INFORMATION, GetFileInformationByHandle}, + }; - let lhs_file = match OpenOptions::new().read(true).share_mode(0).open(lhs) { + let lhs_file = match File::open(lhs) { Ok(f) => f, Err(_) => return false, }; - let result = match OpenOptions::new().read(true).share_mode(0).open(rhs) { - Ok(_) => { - false - } - Err(e) => { - if let Some(error_code) = e.raw_os_error() { - error_code == ERROR_SHARING_VIOLATION - } else { - false - } - } + let rhs_file = match File::open(rhs) { + Ok(f) => f, + Err(_) => return false, }; - drop(lhs_file); + let mut lhs_info = BY_HANDLE_FILE_INFORMATION::default(); + let mut rhs_info = BY_HANDLE_FILE_INFORMATION::default(); + // SAFETY: This is safe because the file handles and the info struct pointers are all valid until this function exits. + unsafe { + if GetFileInformationByHandle(HANDLE(lhs_file.as_raw_handle()), &mut lhs_info).is_err() { + return false; + } - result + if GetFileInformationByHandle(HANDLE(rhs_file.as_raw_handle()), &mut rhs_info).is_err() { + return false; + } + } + + lhs_info.dwVolumeSerialNumber == rhs_info.dwVolumeSerialNumber + && lhs_info.nFileIndexHigh == rhs_info.nFileIndexHigh + && lhs_info.nFileIndexLow == rhs_info.nFileIndexLow } #[cfg(not(windows))]