From c075a7b2d90c377ff2585acd72842e7851a4a387 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Tue, 21 Jul 2026 06:28:58 +0200 Subject: [PATCH] refactor(cli): move destination action policy to Rust FIO_openDstFile retained the filesystem leaf in Rust but its C wrapper still encoded the status precedence for test mode, stdout, same-file protection, sparse-mode adjustment, overwrite prompting, removal, retry, and final errors. Centralize that pure action classification in Rust without moving FILE* handling, metadata, preference mutation, prompts, diagnostics, or retry callbacks across the ABI.\n\nThe action bridge validates status, confirmation, and sparse-state inputs and has focused tests for every destination outcome, sparse-first ordering, prompt acceptance/abort, quiet mode, overwrite mode, setvbuf/open failures, success, and invalid inputs.\n\nTest Plan:\n- ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo fmt/clippy gates passed before staging\n- git diff --cached --check\n- Full capped native and original-test verification follows after the batch is committed. --- programs/fileio.c | 111 ++++++++----- rust/src/fileio_backend.rs | 319 ++++++++++++++++++++++++++++++++++++- 2 files changed, 390 insertions(+), 40 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 8f9979513..b68dddba4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -366,6 +366,28 @@ enum { FIO_RUST_OPEN_DST_NULL_DEVICE_REGULAR = 6, FIO_RUST_OPEN_DST_OPEN_FAILED = 7, }; +enum { + FIO_RUST_OPEN_DST_CONFIRMATION_UNASKED = -1, + FIO_RUST_OPEN_DST_CONFIRMATION_ACCEPT = 0, + FIO_RUST_OPEN_DST_CONFIRMATION_ABORT = 1, +}; +enum { + FIO_RUST_OPEN_DST_ACTION_TEST_MODE = 0, + FIO_RUST_OPEN_DST_ACTION_STDOUT = 1, + FIO_RUST_OPEN_DST_ACTION_SAME_FILE = 2, + FIO_RUST_OPEN_DST_ACTION_ADJUST_SPARSE = 3, + FIO_RUST_OPEN_DST_ACTION_NULL_DEVICE_REGULAR = 4, + FIO_RUST_OPEN_DST_ACTION_EXISTING_QUIET_ABORT = 5, + FIO_RUST_OPEN_DST_ACTION_EXISTING_PROMPT = 6, + FIO_RUST_OPEN_DST_ACTION_EXISTING_ABORT = 7, + FIO_RUST_OPEN_DST_ACTION_EXISTING_REMOVE = 8, + FIO_RUST_OPEN_DST_ACTION_OPEN_FAILED = 9, + FIO_RUST_OPEN_DST_ACTION_SETBUF_FAILED = 10, + FIO_RUST_OPEN_DST_ACTION_SUCCESS = 11, + FIO_RUST_OPEN_DST_ACTION_INVALID = 12, +}; +int FIO_rust_openDstFileAction(int status, int overwrite, int displayLevel, + int confirmation, int sparseAdjusted); int FIO_rust_openDstFile(int testMode, int allowExisting, const char* srcFileName, @@ -690,18 +712,24 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, int isDstRegFile = 0; int allowExisting = 0; int sparseAdjusted = 0; + int confirmation = FIO_RUST_OPEN_DST_CONFIRMATION_UNASKED; int status; FILE* f = NULL; status = FIO_rust_openDstFile(prefs->testMode, allowExisting, srcFileName, dstFileName, mode, &isDstRegFile, &f); - if (status == FIO_RUST_OPEN_DST_TEST_MODE) - return NULL; /* do not open file in test mode */ - - assert(dstFileName != NULL); for (;;) { - if (status == FIO_RUST_OPEN_DST_STDOUT) { + int const action = FIO_rust_openDstFileAction( + status, prefs->overwrite, g_display_prefs.displayLevel, + confirmation, sparseAdjusted); + + if (action == FIO_RUST_OPEN_DST_ACTION_TEST_MODE) + return NULL; /* do not open file in test mode */ + + assert(dstFileName != NULL); + switch (action) { + case FIO_RUST_OPEN_DST_ACTION_STDOUT: DISPLAYLEVEL(4,"Using stdout for output \n"); SET_BINARY_MODE(stdout); if (prefs->sparseFileSupport == 1) { @@ -709,18 +737,15 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, DISPLAYLEVEL(4, "Sparse File Support is automatically disabled on stdout ; try --sparse \n"); } return stdout; - } - - if (status == FIO_RUST_OPEN_DST_SAME_FILE) { + case FIO_RUST_OPEN_DST_ACTION_SAME_FILE: DISPLAYLEVEL(1, "zstd: Refusing to open an output file which will overwrite the input file \n"); return NULL; - } - /* Keep the C-owned preference mutation and its diagnostics here. Do - * this only for the first classification: after an existing file is - * removed, the second Rust call must not reclassify it as a missing - * destination and emit a different sparse-mode message. */ - if (!sparseAdjusted) { + case FIO_RUST_OPEN_DST_ACTION_ADJUST_SPARSE: + /* Rust orders this once-only policy action before all ordinary + * destination outcomes. Keep the preference mutation and its + * diagnostic in C, and let the next loop iteration classify the + * same status again. */ if (prefs->sparseFileSupport == 1) { prefs->sparseFileSupport = ZSTD_SPARSE_DEFAULT; if (!isDstRegFile) { @@ -729,47 +754,61 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, } } sparseAdjusted = 1; - } + continue; - if (status == FIO_RUST_OPEN_DST_NULL_DEVICE_REGULAR) { + case FIO_RUST_OPEN_DST_ACTION_NULL_DEVICE_REGULAR: EXM_THROW(40, "%s is unexpectedly categorized as a regular file", dstFileName); - } + return NULL; - if (status == FIO_RUST_OPEN_DST_EXISTING) { - if (!prefs->overwrite) { - if (g_display_prefs.displayLevel <= 1) { - /* No interaction possible */ - DISPLAYLEVEL(1, "zstd: %s already exists; not overwritten \n", - dstFileName); - return NULL; - } - DISPLAY("zstd: %s already exists; ", dstFileName); - if (UTIL_requireUserConfirmation("overwrite (y/n) ? ", "Not overwritten \n", "yY", fCtx->hasStdinInput)) - return NULL; - } + case FIO_RUST_OPEN_DST_ACTION_EXISTING_QUIET_ABORT: + /* No interaction possible. */ + DISPLAYLEVEL(1, "zstd: %s already exists; not overwritten \n", + dstFileName); + return NULL; + + case FIO_RUST_OPEN_DST_ACTION_EXISTING_PROMPT: + DISPLAY("zstd: %s already exists; ", dstFileName); + confirmation = UTIL_requireUserConfirmation( + "overwrite (y/n) ? ", "Not overwritten \n", "yY", + fCtx->hasStdinInput) + ? FIO_RUST_OPEN_DST_CONFIRMATION_ABORT + : FIO_RUST_OPEN_DST_CONFIRMATION_ACCEPT; + continue; + + case FIO_RUST_OPEN_DST_ACTION_EXISTING_ABORT: + /* UTIL_requireUserConfirmation() already emitted the exact + * existing diagnostic before returning the abort result. */ + return NULL; + + case FIO_RUST_OPEN_DST_ACTION_EXISTING_REMOVE: /* Keep the existing C wrapper so its stat/non-regular diagnostics * remain unchanged. The next Rust call opens with O_TRUNC. */ FIO_removeFile(dstFileName); allowExisting = 1; f = NULL; + confirmation = FIO_RUST_OPEN_DST_CONFIRMATION_UNASKED; status = FIO_rust_openDstFile(prefs->testMode, allowExisting, srcFileName, dstFileName, mode, &isDstRegFile, &f); continue; - } - if (status == FIO_RUST_OPEN_DST_OPEN_FAILED) { + case FIO_RUST_OPEN_DST_ACTION_OPEN_FAILED: DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); return NULL; - } - if (status == FIO_RUST_OPEN_DST_SETBUF_FAILED) + case FIO_RUST_OPEN_DST_ACTION_SETBUF_FAILED: DISPLAYLEVEL(2, "Warning: setvbuf failed for %s\n", dstFileName); + return f; - assert(status == FIO_RUST_OPEN_DST_SUCCESS - || status == FIO_RUST_OPEN_DST_SETBUF_FAILED); - return f; + case FIO_RUST_OPEN_DST_ACTION_SUCCESS: + return f; + + case FIO_RUST_OPEN_DST_ACTION_INVALID: + default: + assert(0); + return f; + } } } diff --git a/rust/src/fileio_backend.rs b/rust/src/fileio_backend.rs index 7683f455f..52665e59c 100644 --- a/rust/src/fileio_backend.rs +++ b/rust/src/fileio_backend.rs @@ -7,10 +7,11 @@ //! This module implements the filesystem operations behind //! `FIO_openDstFile()`, `FIO_openSrcFile()`, `FIO_removeFile()`, and dictionary //! loading, returning small status codes so the C wrappers can retain their -//! existing messages, policy fields, and conventions. Rust also classifies -//! dictionary-loader statuses into diagnostic actions; C retains the -//! metadata and exact diagnostic text while the existing loader leaves -//! continue to publish their narrow status ABI. +//! existing messages, policy fields, and conventions. Rust also owns the +//! pure destination status/action ordering and classifies dictionary-loader +//! statuses into diagnostic actions; C retains the metadata and exact +//! diagnostic text while the existing loader leaves continue to publish their +//! narrow status ABI. use std::ffi::{c_char, c_void, CStr}; use std::fs::File; @@ -64,6 +65,23 @@ const FIO_OPEN_DST_EXISTING: c_int = 5; const FIO_OPEN_DST_NULL_DEVICE_REGULAR: c_int = 6; const FIO_OPEN_DST_OPEN_FAILED: c_int = 7; const FIO_DESTINATION_BUFFER_SIZE: usize = 1 << 20; +const FIO_OPEN_DST_CONFIRMATION_UNASKED: c_int = -1; +const FIO_OPEN_DST_CONFIRMATION_ACCEPT: c_int = 0; +const FIO_OPEN_DST_CONFIRMATION_ABORT: c_int = 1; + +const FIO_OPEN_DST_ACTION_TEST_MODE: c_int = 0; +const FIO_OPEN_DST_ACTION_STDOUT: c_int = 1; +const FIO_OPEN_DST_ACTION_SAME_FILE: c_int = 2; +const FIO_OPEN_DST_ACTION_ADJUST_SPARSE: c_int = 3; +const FIO_OPEN_DST_ACTION_NULL_DEVICE_REGULAR: c_int = 4; +const FIO_OPEN_DST_ACTION_EXISTING_QUIET_ABORT: c_int = 5; +const FIO_OPEN_DST_ACTION_EXISTING_PROMPT: c_int = 6; +const FIO_OPEN_DST_ACTION_EXISTING_ABORT: c_int = 7; +const FIO_OPEN_DST_ACTION_EXISTING_REMOVE: c_int = 8; +const FIO_OPEN_DST_ACTION_OPEN_FAILED: c_int = 9; +const FIO_OPEN_DST_ACTION_SETBUF_FAILED: c_int = 10; +const FIO_OPEN_DST_ACTION_SUCCESS: c_int = 11; +const FIO_OPEN_DST_ACTION_INVALID: c_int = 12; static STDOUT_MARK: &[u8] = b"/*stdout*\\\0"; @@ -103,6 +121,132 @@ fn classify_destination( DestinationDecision::Create } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum DestinationAction { + TestMode, + Stdout, + SameFile, + AdjustSparse, + NullDeviceRegular, + ExistingQuietAbort, + ExistingPrompt, + ExistingAbort, + ExistingRemove, + OpenFailed, + SetbufFailed, + Success, + Invalid, +} + +impl DestinationAction { + fn code(self) -> c_int { + match self { + Self::TestMode => FIO_OPEN_DST_ACTION_TEST_MODE, + Self::Stdout => FIO_OPEN_DST_ACTION_STDOUT, + Self::SameFile => FIO_OPEN_DST_ACTION_SAME_FILE, + Self::AdjustSparse => FIO_OPEN_DST_ACTION_ADJUST_SPARSE, + Self::NullDeviceRegular => FIO_OPEN_DST_ACTION_NULL_DEVICE_REGULAR, + Self::ExistingQuietAbort => FIO_OPEN_DST_ACTION_EXISTING_QUIET_ABORT, + Self::ExistingPrompt => FIO_OPEN_DST_ACTION_EXISTING_PROMPT, + Self::ExistingAbort => FIO_OPEN_DST_ACTION_EXISTING_ABORT, + Self::ExistingRemove => FIO_OPEN_DST_ACTION_EXISTING_REMOVE, + Self::OpenFailed => FIO_OPEN_DST_ACTION_OPEN_FAILED, + Self::SetbufFailed => FIO_OPEN_DST_ACTION_SETBUF_FAILED, + Self::Success => FIO_OPEN_DST_ACTION_SUCCESS, + Self::Invalid => FIO_OPEN_DST_ACTION_INVALID, + } + } +} + +fn is_known_destination_status(status: c_int) -> bool { + matches!( + status, + FIO_OPEN_DST_SUCCESS + | FIO_OPEN_DST_SETBUF_FAILED + | FIO_OPEN_DST_TEST_MODE + | FIO_OPEN_DST_STDOUT + | FIO_OPEN_DST_SAME_FILE + | FIO_OPEN_DST_EXISTING + | FIO_OPEN_DST_NULL_DEVICE_REGULAR + | FIO_OPEN_DST_OPEN_FAILED + ) +} + +/// Selects the next destination-opening action without touching filesystem or +/// CLI state. +/// +/// Sparse-mode adjustment is returned as its own first action because C owns +/// the preference field and its diagnostic. C calls back with +/// `sparse_adjusted = 1` after applying that mutation, then this same policy +/// selects the status-specific action. Likewise, C owns the confirmation +/// callback and supplies its result on the next call. This keeps the retry +/// mechanics, FILE* ownership, metadata, and exact diagnostics in C while the +/// ordering between those operations is centralized here. +fn destination_action( + status: c_int, + overwrite: c_int, + display_level: c_int, + confirmation: c_int, + sparse_adjusted: c_int, +) -> DestinationAction { + if !is_known_destination_status(status) + || (sparse_adjusted != 0 && sparse_adjusted != 1) + { + DestinationAction::Invalid + } else if sparse_adjusted == 0 + && !matches!( + status, + FIO_OPEN_DST_TEST_MODE | FIO_OPEN_DST_STDOUT | FIO_OPEN_DST_SAME_FILE + ) + { + DestinationAction::AdjustSparse + } else { + match status { + FIO_OPEN_DST_TEST_MODE => DestinationAction::TestMode, + FIO_OPEN_DST_STDOUT => DestinationAction::Stdout, + FIO_OPEN_DST_SAME_FILE => DestinationAction::SameFile, + FIO_OPEN_DST_NULL_DEVICE_REGULAR => DestinationAction::NullDeviceRegular, + FIO_OPEN_DST_EXISTING => { + if overwrite != 0 { + DestinationAction::ExistingRemove + } else if display_level <= 1 { + DestinationAction::ExistingQuietAbort + } else { + match confirmation { + FIO_OPEN_DST_CONFIRMATION_UNASKED => DestinationAction::ExistingPrompt, + FIO_OPEN_DST_CONFIRMATION_ACCEPT => DestinationAction::ExistingRemove, + FIO_OPEN_DST_CONFIRMATION_ABORT => DestinationAction::ExistingAbort, + _ => DestinationAction::Invalid, + } + } + } + FIO_OPEN_DST_OPEN_FAILED => DestinationAction::OpenFailed, + FIO_OPEN_DST_SETBUF_FAILED => DestinationAction::SetbufFailed, + FIO_OPEN_DST_SUCCESS => DestinationAction::Success, + _ => DestinationAction::Invalid, + } + } +} + +/// Exposes the pure destination status/action policy to the C adapter. +#[no_mangle] +pub extern "C" fn FIO_rust_openDstFileAction( + status: c_int, + overwrite: c_int, + display_level: c_int, + confirmation: c_int, + sparse_adjusted: c_int, +) -> c_int { + destination_action( + status, + overwrite, + display_level, + confirmation, + sparse_adjusted, + ) + .code() +} + unsafe extern "C" { fn UTIL_stat(file_name: *const c_char, stat_buf: *mut libc::stat) -> c_int; fn UTIL_isRegularFileStat(stat_buf: *const libc::stat) -> c_int; @@ -891,6 +1035,173 @@ mod tests { ); } + #[test] + fn destination_open_action_covers_status_ordering_and_confirmation_branches() { + let cases = [ + ( + FIO_OPEN_DST_TEST_MODE, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 0, + DestinationAction::TestMode, + ), + ( + FIO_OPEN_DST_STDOUT, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 0, + DestinationAction::Stdout, + ), + ( + FIO_OPEN_DST_SAME_FILE, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 0, + DestinationAction::SameFile, + ), + ( + FIO_OPEN_DST_NULL_DEVICE_REGULAR, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 0, + DestinationAction::AdjustSparse, + ), + ( + FIO_OPEN_DST_NULL_DEVICE_REGULAR, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::NullDeviceRegular, + ), + ( + FIO_OPEN_DST_EXISTING, + 0, + 1, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::ExistingQuietAbort, + ), + ( + FIO_OPEN_DST_EXISTING, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::ExistingPrompt, + ), + ( + FIO_OPEN_DST_EXISTING, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_ABORT, + 1, + DestinationAction::ExistingAbort, + ), + ( + FIO_OPEN_DST_EXISTING, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_ACCEPT, + 1, + DestinationAction::ExistingRemove, + ), + ( + FIO_OPEN_DST_EXISTING, + 1, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::ExistingRemove, + ), + ( + FIO_OPEN_DST_OPEN_FAILED, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::OpenFailed, + ), + ( + FIO_OPEN_DST_SETBUF_FAILED, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::SetbufFailed, + ), + ( + FIO_OPEN_DST_SUCCESS, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 1, + DestinationAction::Success, + ), + ]; + + for (status, overwrite, display_level, confirmation, sparse_adjusted, expected) in cases { + assert_eq!( + destination_action( + status, + overwrite, + display_level, + confirmation, + sparse_adjusted, + ), + expected, + "unexpected destination action for status {status}" + ); + assert_eq!( + FIO_rust_openDstFileAction( + status, + overwrite, + display_level, + confirmation, + sparse_adjusted, + ), + expected.code(), + "C action ABI disagrees for status {status}" + ); + } + } + + #[test] + fn destination_open_action_rejects_invalid_policy_inputs() { + assert_eq!( + destination_action( + c_int::MAX, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 0, + ), + DestinationAction::Invalid + ); + assert_eq!( + destination_action( + FIO_OPEN_DST_SUCCESS, + 0, + 2, + FIO_OPEN_DST_CONFIRMATION_UNASKED, + 2, + ), + DestinationAction::Invalid + ); + assert_eq!( + destination_action(FIO_OPEN_DST_EXISTING, 0, 2, c_int::MAX, 1), + DestinationAction::Invalid + ); + assert_eq!( + FIO_rust_openDstFileAction(c_int::MAX, 0, 2, -1, 0), + FIO_OPEN_DST_ACTION_INVALID + ); + } + #[test] fn dictionary_loader_diagnostics_preserve_each_backend_status_class() { let cases = [