From 386ec3d0a73ae32992f9b4ff90f9660f067aaad7 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 18 Jul 2026 03:39:22 +0200 Subject: [PATCH] feat(fileio): move size helpers to Rust Move the file-stat size scan and highest-bit helper behind Rust ABI implementations while preserving the C file-I/O surface and unknown-size sentinel behavior. Add temporary-file and boundary coverage for the migrated helpers. Test Plan: cargo test --manifest-path rust/cli/Cargo.toml --no-default-features --features cli,compression,decompression,benchmark; cargo test --manifest-path rust/cli/Cargo.toml --no-default-features --features helpers; cargo clippy --manifest-path rust/cli/Cargo.toml --all-targets --no-default-features --features cli,compression,decompression,benchmark; cargo clippy --manifest-path rust/cli/Cargo.toml --all-targets --no-default-features --features helpers; make -B -C programs -j2 zstd zstd-small zstd-frugal; make -B -C tests -j2 test-cli-tests --- programs/fileio.c | 15 ++----- rust/src/fileio_prefs.rs | 87 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 436f24862..3a278125e 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -307,6 +307,8 @@ static int FIO_shouldDisplayMultipleFileSummary(FIO_ctx_t const* fCtx) /* 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. */ 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); /*-************************************* @@ -702,11 +704,8 @@ FIO_createFilename_fromOutDir(const char* path, const char* outDirName, const si */ static unsigned FIO_highbit64(unsigned long long v) { - unsigned count = 0; assert(v != 0); - v >>= 1; - while (v) { v >>= 1; count++; } - return count; + return FIO_rust_highbit64(v); } static void FIO_adjustMemLimitForPatchFromMode(FIO_prefs_t* const prefs, @@ -1948,13 +1947,7 @@ FIO_determineCompressedName(const char* srcFileName, const char* outDirName, con static unsigned long long FIO_getLargestFileSize(const char** inFileNames, unsigned nbFiles) { - size_t i; - unsigned long long fileSize, maxFileSize = 0; - for (i = 0; i < nbFiles; i++) { - fileSize = UTIL_getFileSize(inFileNames[i]); - maxFileSize = fileSize > maxFileSize ? fileSize : maxFileSize; - } - return maxFileSize; + return FIO_rust_getLargestFileSize(inFileNames, nbFiles); } /* FIO_compressMultipleFilenames() : diff --git a/rust/src/fileio_prefs.rs b/rust/src/fileio_prefs.rs index bb58cebe6..d88dc08ef 100644 --- a/rust/src/fileio_prefs.rs +++ b/rust/src/fileio_prefs.rs @@ -476,6 +476,47 @@ pub unsafe extern "C" fn FIO_determineHasStdinInput( } } +/// Return the position of the highest set bit in a non-zero 64-bit value. +/// +/// This mirrors `FIO_highbit64()` and deliberately keeps its zero-input +/// assertion semantics. In builds where assertions are disabled, zero +/// still produces the same result as the original shift loop. +#[no_mangle] +pub extern "C" fn FIO_rust_highbit64(value: u64) -> c_uint { + debug_assert!(value != 0); + if value == 0 { + return 0; + } + (u64::BITS - 1 - value.leading_zeros()) as c_uint +} + +fn largest_file_size(sizes: I) -> u64 +where + I: IntoIterator, +{ + let mut largest = 0; + for size in sizes { + largest = size.max(largest); + } + largest +} + +/// Rust implementation of the size scan used by `FIO_getLargestFileSize`. +/// +/// `UTIL_getFileSize()` remains responsible for the actual stat operation. +/// In particular, an unknown size is `UTIL_FILESIZE_UNKNOWN` and therefore +/// wins the max scan exactly as it does in the original C implementation. +#[no_mangle] +pub unsafe extern "C" fn FIO_rust_getLargestFileSize( + file_names: *const *const c_char, + nb_files: c_uint, +) -> u64 { + largest_file_size( + (0..nb_files as usize) + .map(|index| unsafe { crate::util::UTIL_getFileSize(*file_names.add(index)) }), + ) +} + #[inline] fn filename_path_separator() -> u8 { if cfg!(windows) { @@ -561,8 +602,54 @@ pub unsafe extern "C" fn FIO_rust_checkFilenameCollisions( mod tests { use super::*; use std::ffi::CString; + use std::fs; use std::mem::{align_of, offset_of, size_of}; + const UTIL_FILESIZE_UNKNOWN: u64 = u64::MAX; + + fn temporary_file_path(name: &str) -> std::path::PathBuf { + std::env::temp_dir().join(format!("zstd-fileio-prefs-{}-{name}", std::process::id())) + } + + #[test] + fn largest_file_size_scan_handles_empty_input() { + assert_eq!(unsafe { FIO_rust_getLargestFileSize(ptr::null(), 0) }, 0); + } + + #[test] + fn largest_file_size_scan_handles_ordinary_files() { + let smaller_path = temporary_file_path("smaller"); + let larger_path = temporary_file_path("larger"); + fs::write(&smaller_path, b"small").unwrap(); + fs::write(&larger_path, b"larger file").unwrap(); + let smaller = CString::new(smaller_path.to_string_lossy().as_bytes()).unwrap(); + let larger = CString::new(larger_path.to_string_lossy().as_bytes()).unwrap(); + let names = [smaller.as_ptr(), larger.as_ptr()]; + + assert_eq!( + unsafe { FIO_rust_getLargestFileSize(names.as_ptr(), names.len() as c_uint) }, + 11 + ); + + fs::remove_file(smaller_path).unwrap(); + fs::remove_file(larger_path).unwrap(); + } + + #[test] + fn largest_file_size_scan_preserves_large_values_and_errors() { + assert_eq!(largest_file_size([0, 1u64 << 63, 7]), 1u64 << 63); + assert_eq!(FIO_rust_highbit64(1u64 << 63), 63); + assert_eq!(FIO_rust_highbit64(u64::MAX), 63); + + let missing_path = temporary_file_path("missing"); + let missing = CString::new(missing_path.to_string_lossy().as_bytes()).unwrap(); + let names = [missing.as_ptr()]; + assert_eq!( + unsafe { FIO_rust_getLargestFileSize(names.as_ptr(), 1) }, + UTIL_FILESIZE_UNKNOWN + ); + } + #[test] fn c_layouts_match_the_headers() { let word = size_of::();