From 52c0eb4b9b77663298dadd62427ad169f0c76e7b Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 18 Jul 2026 05:39:21 +0200 Subject: [PATCH] feat(cli): move regular-file removal to Rust Route the CLI backend's safe file-removal leaf through a narrow Rust status ABI while preserving C diagnostics, Windows read-only handling, and the existing return convention. Test Plan: cargo test --manifest-path rust/cli/Cargo.toml; cargo clippy --manifest-path rust/cli/Cargo.toml; cargo clippy --manifest-path rust/cli/Cargo.toml --benches; cargo clippy --manifest-path rust/cli/Cargo.toml --tests; make -B -C programs -j2 zstd; make -C tests -j2 test-cli-tests (41 tests); make -B -C programs -j2 zstd-small zstd-frugal. --- programs/Makefile | 1 + programs/fileio.c | 26 ++++---- rust/cli/src/lib.rs | 3 + rust/src/fileio_backend.rs | 121 +++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 rust/src/fileio_backend.rs diff --git a/programs/Makefile b/programs/Makefile index f8b42893c..7df1d0467 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -35,6 +35,7 @@ RUST_CLI_SOURCES := $(RUST_CLI_MANIFEST) $(RUST_CLI_DIR)/Cargo.lock \ $(RUST_CLI_DIR)/src/lib.rs $(RUST_DIR)/src/zstd_cli.rs \ $(RUST_DIR)/src/benchzstd.rs \ $(RUST_DIR)/src/fileio_prefs.rs \ + $(RUST_DIR)/src/fileio_backend.rs \ $(RUST_DIR)/src/dibio.rs \ $(RUST_DIR)/src/util.rs \ $(RUST_DIR)/src/zstdcli_trace.rs \ diff --git a/programs/fileio.c b/programs/fileio.c index a8ffcb50e..22c0f0e58 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -305,8 +305,9 @@ static int FIO_shouldDisplayMultipleFileSummary(FIO_ctx_t const* fCtx) #define FIO_LDM_PARAM_NOTSET 9999 -/* These symbols are implemented by rust/src/fileio_prefs.rs. The declarations - * in fileio.h remain the C ABI shims while all actual file I/O stays here. */ +/* These policy and filesystem leaves are implemented by the Rust CLI archive. + * The declarations in fileio.h remain the C ABI shims while file-I/O + * orchestration and diagnostics stay here. */ int FIO_rust_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles); unsigned FIO_rust_highbit64(unsigned long long v); unsigned long long FIO_rust_getLargestFileSize(const char** inFileNames, unsigned nbFiles); @@ -319,6 +320,7 @@ const char* FIO_rust_determineDstName(const char* srcFileName, const char* outDi int FIO_rust_adjustMemLimitForPatchFromMode(FIO_prefs_t* prefs, unsigned long long dictSize, unsigned long long maxSrcFileSize); +int FIO_rust_removeFile(const char* path); #ifdef ZSTD_LZ4COMPRESS int FIO_rust_LZ4_GetBlockSize_FromBlockId(int id); #endif @@ -343,23 +345,21 @@ int FIO_rust_LZ4_GetBlockSize_FromBlockId(int id); * @result : Unlink `fileName`, even if it's read-only */ static int FIO_removeFile(const char* path) { - stat_t statbuf; - if (!UTIL_stat(path, &statbuf)) { + enum { + FIO_REMOVE_SUCCESS = 0, + FIO_REMOVE_STAT_FAILED = 1, + FIO_REMOVE_NON_REGULAR = 2, + }; + int const status = FIO_rust_removeFile(path); + if (status == FIO_REMOVE_STAT_FAILED) { DISPLAYLEVEL(2, "zstd: Failed to stat %s while trying to remove it\n", path); return 0; } - if (!UTIL_isRegularFileStat(&statbuf)) { + if (status == FIO_REMOVE_NON_REGULAR) { DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s\n", path); return 0; } -#if defined(_WIN32) - /* windows doesn't allow remove read-only files, - * so try to make it writable first */ - if (!(statbuf.st_mode & _S_IWRITE)) { - UTIL_chmod(path, &statbuf, _S_IWRITE); - } -#endif - return remove(path); + return status == FIO_REMOVE_SUCCESS ? 0 : 1; } /** FIO_openSrcFile() : diff --git a/rust/cli/src/lib.rs b/rust/cli/src/lib.rs index b4548f622..1600f404e 100644 --- a/rust/cli/src/lib.rs +++ b/rust/cli/src/lib.rs @@ -11,6 +11,9 @@ mod datagen; #[path = "../../src/dibio.rs"] mod dibio; #[cfg(feature = "cli")] +#[path = "../../src/fileio_backend.rs"] +mod fileio_backend; +#[cfg(feature = "cli")] #[path = "../../src/fileio_prefs.rs"] mod fileio_prefs; #[cfg(any(feature = "benchmark", feature = "helpers"))] diff --git a/rust/src/fileio_backend.rs b/rust/src/fileio_backend.rs new file mode 100644 index 000000000..2d8f33b51 --- /dev/null +++ b/rust/src/fileio_backend.rs @@ -0,0 +1,121 @@ +#![allow(non_snake_case)] +#![allow(clippy::missing_safety_doc)] + +//! Rust-owned filesystem leaves for the command-line backend. +//! +//! The surrounding CLI still owns policy, diagnostics, and stream orchestration +//! in C. This module only implements the filesystem operation behind +//! `FIO_removeFile()`, returning a small status code so the C wrapper can retain +//! its existing messages and return convention. + +use std::ffi::{c_char, CStr}; +use std::os::raw::c_int; +use std::path::{Path, PathBuf}; + +const FIO_REMOVE_SUCCESS: c_int = 0; +const FIO_REMOVE_STAT_FAILED: c_int = 1; +const FIO_REMOVE_NON_REGULAR: c_int = 2; +const FIO_REMOVE_FAILED: c_int = 3; + +fn path_from_c(path: *const c_char) -> Option { + if path.is_null() { + return None; + } + let path = unsafe { CStr::from_ptr(path) }; + #[cfg(unix)] + { + use std::os::unix::ffi::OsStrExt; + Some(Path::new(std::ffi::OsStr::from_bytes(path.to_bytes())).to_path_buf()) + } + #[cfg(not(unix))] + { + Some(PathBuf::from(path.to_string_lossy().into_owned())) + } +} + +/// Removes a regular file and returns a status consumed by the C adapter. +/// +/// The metadata query follows symlinks, matching the existing `stat()`-based +/// C helper. On Windows, read-only files are made writable before removal; +/// failure to change the permission is intentionally ignored, just as the C +/// implementation ignored `UTIL_chmod()`'s result. +#[no_mangle] +pub unsafe extern "C" fn FIO_rust_removeFile(path: *const c_char) -> c_int { + let Some(path) = path_from_c(path) else { + return FIO_REMOVE_STAT_FAILED; + }; + + let metadata = match std::fs::metadata(&path) { + Ok(metadata) => metadata, + Err(_) => return FIO_REMOVE_STAT_FAILED, + }; + if !metadata.file_type().is_file() { + return FIO_REMOVE_NON_REGULAR; + } + + #[cfg(windows)] + { + let mut permissions = metadata.permissions(); + if permissions.readonly() { + permissions.set_readonly(false); + let _ = std::fs::set_permissions(&path, permissions); + } + } + + match std::fs::remove_file(path) { + Ok(()) => FIO_REMOVE_SUCCESS, + Err(_) => FIO_REMOVE_FAILED, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::ffi::CString; + use std::fs; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::time::{SystemTime, UNIX_EPOCH}; + + static NEXT_TEMP_ID: AtomicUsize = AtomicUsize::new(0); + + fn temp_path(label: &str) -> PathBuf { + let time = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock before epoch") + .as_nanos(); + let id = NEXT_TEMP_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "zstd-rust-fileio-{label}-{}-{time}-{id}", + std::process::id() + )) + } + + fn c_path(path: &Path) -> CString { + CString::new(path.to_string_lossy().as_bytes()).expect("temporary path contains NUL") + } + + #[test] + fn removes_regular_file() { + let path = temp_path("regular"); + fs::write(&path, b"payload").expect("create temporary file"); + let result = unsafe { FIO_rust_removeFile(c_path(&path).as_ptr()) }; + assert_eq!(result, FIO_REMOVE_SUCCESS); + assert!(!path.exists()); + } + + #[test] + fn rejects_missing_path_as_stat_failure() { + let path = temp_path("missing"); + let result = unsafe { FIO_rust_removeFile(c_path(&path).as_ptr()) }; + assert_eq!(result, FIO_REMOVE_STAT_FAILED); + } + + #[test] + fn rejects_directory_as_non_regular() { + let path = temp_path("directory"); + fs::create_dir(&path).expect("create temporary directory"); + let result = unsafe { FIO_rust_removeFile(c_path(&path).as_ptr()) }; + assert_eq!(result, FIO_REMOVE_NON_REGULAR); + fs::remove_dir(&path).expect("remove temporary directory"); + } +}