ddidderr/zstd-rs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(cli): move regular-file removal to Rust

Browse Source 
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.
This commit is contained in:
ddidderr
2026-07-18 05:39:21 +02:00
parent 327272f8b3
commit 52c0eb4b9b
4 changed files with 138 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
programs/Makefile
+1
View File
@@ -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 \
programs/fileio.c
+13 -13
View File
@@ -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() :
rust/cli/src/lib.rs
+3
View File
@@ -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"))]
rust/src/fileio_backend.rs
+121
View File
@@ -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<PathBuf> {
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");
}
}