From 2e5f7308c82f038d82fbcbfb27bf5cd319a06424 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Tue, 21 Jul 2026 09:48:48 +0200 Subject: [PATCH] refactor(fileio): move list-file ordering to Rust Move the single-file --list status gates, diagnostic-versus-metadata ordering, and aggregate projection into Rust. C retains file opening and frame analysis, the private fileInfo_t layout, exact diagnostics, and row formatting behind explicit callbacks, so the user-visible behavior remains unchanged while the policy boundary is auditable. Test Plan: - git diff --cached --check - worker format, check, clippy, and serial C compilation (passed); focused Rust tests compiled but the standalone link hit the pre-existing C bridge-symbol gap --- programs/fileio.c | 120 ++++++++++------ rust/src/fileio_prefs.rs | 296 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 372 insertions(+), 44 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 787ab7cc6..d5991c926 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -4781,8 +4781,8 @@ typedef enum { info_truncated_input=4 } InfoError; -/* Rust owns the scalar --list file-status action policy. C keeps the exact - * diagnostics, metadata formatting, file I/O, and private fileInfo_t layout. */ +/* Rust owns the --list file policy/order. C keeps the exact diagnostics, + * metadata formatting, file I/O, and private fileInfo_t layout. */ enum { FIO_RUST_LIST_FILE_ACTION_SUCCESS = 0, FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR = 1, @@ -4795,7 +4795,7 @@ int FIO_rust_listFileStatusAction(int status); /* Keep the private fileInfo_t layout in C. This projection contains only the * fields needed by the --list total row and crosses the Rust policy boundary - * after FIO_listFile has finished its C-owned open/parse/display work. */ + * after the C-owned file-info and display callbacks have run. */ typedef struct { U64 decompressedSize; U64 compressedSize; @@ -4806,6 +4806,35 @@ typedef struct { U32 nbFiles; } FIO_listFileInfoProjection_t; +typedef int (*FIO_listFileGetInfoFn)(void* opaque, const char* fileName, + fileInfo_t* info); +typedef void (*FIO_listFileDisplayStatusFn)(void* opaque, int action, + const char* fileName, + int displayLevel); +typedef void (*FIO_listFileDisplayInfoFn)(void* opaque, const char* fileName, + const fileInfo_t* info, + int displayLevel); + +/* This callback state is a deliberate repr(C) bridge: filesystem access, + * frame analysis, exact diagnostics, and row formatting stay in C while Rust + * owns the per-file policy/order around them. */ +typedef struct { + void* opaque; + FIO_listFileGetInfoFn getInfo; + FIO_listFileDisplayStatusFn displayStatus; + FIO_listFileDisplayInfoFn displayInfo; +} FIO_listFileCallbacks_t; + +typedef char FIO_rust_list_file_callbacks_get_info_offset[ + (offsetof(FIO_listFileCallbacks_t, getInfo) == sizeof(void*)) ? 1 : -1]; +typedef char FIO_rust_list_file_callbacks_status_offset[ + (offsetof(FIO_listFileCallbacks_t, displayStatus) + == sizeof(void*) + sizeof(FIO_listFileGetInfoFn)) ? 1 : -1]; +typedef char FIO_rust_list_file_callbacks_info_offset[ + (offsetof(FIO_listFileCallbacks_t, displayInfo) + == sizeof(void*) + sizeof(FIO_listFileGetInfoFn) + + sizeof(FIO_listFileDisplayStatusFn)) ? 1 : -1]; + typedef struct { void* opaque; int (*isStdin)(void* opaque, const char* fileName); @@ -4818,6 +4847,9 @@ typedef struct { const FIO_listFileInfoProjection_t* total); } FIO_listMultipleFilesCallbacks_t; +int FIO_rust_listFile(const char* inputName, int displayLevel, + FIO_listFileInfoProjection_t* output, + const FIO_listFileCallbacks_t* callbacks); int FIO_rust_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel, const FIO_listMultipleFilesCallbacks_t* callbacks); @@ -4987,49 +5019,48 @@ displayInfo(const char* inFileName, const fileInfo_t* info, int displayLevel) } static int -FIO_listFile(const char* inFileName, int displayLevel, - FIO_listFileInfoProjection_t* output) +FIO_listFileGetInfo(void* opaque, const char* inFileName, fileInfo_t* info) { - fileInfo_t info; - memset(&info, 0, sizeof(info)); - { InfoError const error = getFileInfo(&info, inFileName); - int const action = FIO_rust_listFileStatusAction((int)error); - switch (action) { - case FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR: - /* display error, but provide output */ - DISPLAYLEVEL(1, "Error while parsing \"%s\" \n", inFileName); - break; - case FIO_RUST_LIST_FILE_ACTION_NOT_ZSTD: - DISPLAYOUT("File \"%s\" not compressed by zstd \n", inFileName); - if (displayLevel > 2) DISPLAYOUT("\n"); - return 1; - case FIO_RUST_LIST_FILE_ACTION_FILE_ERROR: - /* error occurred while opening the file */ - if (displayLevel > 2) DISPLAYOUT("\n"); - return 1; - case FIO_RUST_LIST_FILE_ACTION_TRUNCATED_INPUT: - DISPLAYOUT("File \"%s\" is truncated \n", inFileName); - if (displayLevel > 2) DISPLAYOUT("\n"); - return 1; - case FIO_RUST_LIST_FILE_ACTION_SUCCESS: - default: - break; - } + (void)opaque; + return (int)getFileInfo(info, inFileName); +} - displayInfo(inFileName, &info, displayLevel); - output->decompressedSize = info.decompressedSize; - output->compressedSize = info.compressedSize; - output->numActualFrames = info.numActualFrames; - output->numSkippableFrames = info.numSkippableFrames; - output->decompUnavailable = info.decompUnavailable; - output->usesCheck = info.usesCheck; - output->nbFiles = info.nbFiles; - assert(action == FIO_RUST_LIST_FILE_ACTION_SUCCESS - || action == FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR); - return (int)error; +static void +FIO_listFileDisplayStatus(void* opaque, int action, + const char* inFileName, int displayLevel) +{ + (void)opaque; + switch (action) { + case FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR: + /* display error, but provide output */ + DISPLAYLEVEL(1, "Error while parsing \"%s\" \n", inFileName); + break; + case FIO_RUST_LIST_FILE_ACTION_NOT_ZSTD: + DISPLAYOUT("File \"%s\" not compressed by zstd \n", inFileName); + if (displayLevel > 2) DISPLAYOUT("\n"); + break; + case FIO_RUST_LIST_FILE_ACTION_FILE_ERROR: + /* error occurred while opening the file */ + if (displayLevel > 2) DISPLAYOUT("\n"); + break; + case FIO_RUST_LIST_FILE_ACTION_TRUNCATED_INPUT: + DISPLAYOUT("File \"%s\" is truncated \n", inFileName); + if (displayLevel > 2) DISPLAYOUT("\n"); + break; + default: + assert(0); + break; } } +static void +FIO_listFileDisplayInfo(void* opaque, const char* inFileName, + const fileInfo_t* info, int displayLevel) +{ + (void)opaque; + displayInfo(inFileName, info, displayLevel); +} + /* Rust owns this stateless --list input-name predicate. */ int FIO_listFileIsStdin(void* opaque, const char* fileName); @@ -5062,9 +5093,14 @@ static int FIO_listFileCallback(void* opaque, const char* fileName, int displayLevel, FIO_listFileInfoProjection_t* info) { + FIO_listFileCallbacks_t callbacks; (void)opaque; memset(info, 0, sizeof(*info)); - return FIO_listFile(fileName, displayLevel, info); + callbacks.opaque = NULL; + callbacks.getInfo = FIO_listFileGetInfo; + callbacks.displayStatus = FIO_listFileDisplayStatus; + callbacks.displayInfo = FIO_listFileDisplayInfo; + return FIO_rust_listFile(fileName, displayLevel, info, &callbacks); } static void diff --git a/rust/src/fileio_prefs.rs b/rust/src/fileio_prefs.rs index 4a21db278..e3a513ed8 100644 --- a/rust/src/fileio_prefs.rs +++ b/rust/src/fileio_prefs.rs @@ -241,6 +241,7 @@ pub struct FIO_compressionParameters { /// C's private `fileInfo_t` from `programs/fileio.c`. #[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct FIO_fileInfo_t { decompressedSize: u64, compressedSize: u64, @@ -255,8 +256,8 @@ pub struct FIO_fileInfo_t { } /// The subset of C's private `fileInfo_t` needed by the `--list` aggregate -/// row. C keeps opening, parsing, and formatting each file; Rust owns only -/// the policy that iterates and aggregates these values. +/// row. C keeps opening, parsing, and formatting each file; Rust owns the +/// per-file and multi-file policy around those callbacks. #[repr(C)] #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct FIO_listFileInfoProjection_t { @@ -729,6 +730,33 @@ pub type FIO_listMultipleFilesListFileFn = unsafe extern "C" fn( pub type FIO_listMultipleFilesDisplayTotalFn = unsafe extern "C" fn(*mut c_void, *const FIO_listFileInfoProjection_t); +pub type FIO_listFileGetInfoFn = unsafe extern "C" fn( + *mut c_void, + *const c_char, + *mut FIO_fileInfo_t, +) -> c_int; +pub type FIO_listFileDisplayStatusFn = + unsafe extern "C" fn(*mut c_void, c_int, *const c_char, c_int); +pub type FIO_listFileDisplayInfoFn = unsafe extern "C" fn( + *mut c_void, + *const c_char, + *const FIO_fileInfo_t, + c_int, +); + +/// Callbacks for the C-owned `--list` file operation. +/// +/// The callbacks keep stat/open, frame analysis, exact status diagnostics, and +/// row formatting in C. Rust only orders those operations and publishes the +/// small aggregate projection consumed by the surrounding multi-file policy. +#[repr(C)] +pub struct FIO_listFileCallbacks_t { + opaque: *mut c_void, + getInfo: Option, + displayStatus: Option, + displayInfo: Option, +} + #[inline] fn list_file_status_action(status: c_int) -> c_int { match status { @@ -750,6 +778,85 @@ pub extern "C" fn FIO_rust_listFileStatusAction(status: c_int) -> c_int { list_file_status_action(status) } +#[inline] +fn project_list_file_info(info: &FIO_fileInfo_t) -> FIO_listFileInfoProjection_t { + FIO_listFileInfoProjection_t { + decompressedSize: info.decompressedSize, + compressedSize: info.compressedSize, + numActualFrames: info.numActualFrames, + numSkippableFrames: info.numSkippableFrames, + decompUnavailable: info.decompUnavailable, + usesCheck: info.usesCheck, + nbFiles: info.nbFiles, + } +} + +/// Run the policy and ordering around one C-owned `--list` file operation. +/// +/// C retains the stat/open and frame-analysis callback, all user-visible +/// status diagnostics, and the full metadata row formatter. Rust preserves +/// the original status gates: fatal statuses are reported and return `1`, a +/// frame error reports a diagnostic but still formats and publishes its +/// partial metadata, and success formats and publishes normally. +#[no_mangle] +pub unsafe extern "C" fn FIO_rust_listFile( + input_name: *const c_char, + display_level: c_int, + output: *mut FIO_listFileInfoProjection_t, + callbacks: *const FIO_listFileCallbacks_t, +) -> c_int { + assert!(!input_name.is_null()); + assert!(!output.is_null()); + assert!(!callbacks.is_null()); + + let callbacks = unsafe { &*callbacks }; + let get_info = callbacks + .getInfo + .expect("--list file-info callback is required"); + let display_status = callbacks + .displayStatus + .expect("--list status-display callback is required"); + let display_info = callbacks + .displayInfo + .expect("--list info-display callback is required"); + + let mut info = unsafe { std::mem::zeroed::() }; + let status = unsafe { get_info(callbacks.opaque, input_name, &mut info) }; + let action = list_file_status_action(status); + + match action { + FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR + | FIO_RUST_LIST_FILE_ACTION_NOT_ZSTD + | FIO_RUST_LIST_FILE_ACTION_FILE_ERROR + | FIO_RUST_LIST_FILE_ACTION_TRUNCATED_INPUT => unsafe { + display_status(callbacks.opaque, action, input_name, display_level); + }, + FIO_RUST_LIST_FILE_ACTION_SUCCESS | FIO_RUST_LIST_FILE_ACTION_INVALID => {} + _ => unreachable!("unknown --list file action {action}"), + } + + match action { + FIO_RUST_LIST_FILE_ACTION_NOT_ZSTD + | FIO_RUST_LIST_FILE_ACTION_FILE_ERROR + | FIO_RUST_LIST_FILE_ACTION_TRUNCATED_INPUT => 1, + FIO_RUST_LIST_FILE_ACTION_SUCCESS | FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR => { + unsafe { display_info(callbacks.opaque, input_name, &info, display_level) }; + unsafe { output.write(project_list_file_info(&info)) }; + status + } + FIO_RUST_LIST_FILE_ACTION_INVALID => { + unsafe { display_info(callbacks.opaque, input_name, &info, display_level) }; + unsafe { output.write(project_list_file_info(&info)) }; + debug_assert!( + action == FIO_RUST_LIST_FILE_ACTION_SUCCESS + || action == FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR + ); + status + } + _ => unreachable!("unknown --list file action {action}"), + } +} + #[inline] fn remove_file_action(status: c_int) -> c_int { match status { @@ -3296,6 +3403,57 @@ mod tests { } } + #[derive(Default)] + struct ListFileTestState { + status: c_int, + info: FIO_fileInfo_t, + events: Vec<&'static str>, + status_actions: Vec<(c_int, c_int)>, + displayed: Option<(FIO_fileInfo_t, c_int)>, + } + + unsafe extern "C" fn list_file_get_info( + opaque: *mut c_void, + _file_name: *const c_char, + info: *mut FIO_fileInfo_t, + ) -> c_int { + let state = unsafe { &mut *opaque.cast::() }; + state.events.push("get-info"); + unsafe { info.write(state.info) }; + state.status + } + + unsafe extern "C" fn list_file_display_status( + opaque: *mut c_void, + action: c_int, + _file_name: *const c_char, + display_level: c_int, + ) { + let state = unsafe { &mut *opaque.cast::() }; + state.events.push("display-status"); + state.status_actions.push((action, display_level)); + } + + unsafe extern "C" fn list_file_display_info( + opaque: *mut c_void, + _file_name: *const c_char, + info: *const FIO_fileInfo_t, + display_level: c_int, + ) { + let state = unsafe { &mut *opaque.cast::() }; + state.events.push("display-info"); + state.displayed = Some((unsafe { *info }, display_level)); + } + + fn list_file_callbacks(state: &mut ListFileTestState) -> FIO_listFileCallbacks_t { + FIO_listFileCallbacks_t { + opaque: (state as *mut ListFileTestState).cast(), + getInfo: Some(list_file_get_info), + displayStatus: Some(list_file_display_status), + displayInfo: Some(list_file_display_info), + } + } + #[cfg(feature = "decompression")] struct TemporaryCFile(*mut libc::FILE); @@ -4031,6 +4189,140 @@ mod tests { ); } + #[test] + fn list_file_policy_preserves_status_order_and_projection_gates() { + let file_name = CString::new("input.zst").unwrap(); + let info = FIO_fileInfo_t { + decompressedSize: 100, + compressedSize: 40, + windowSize: 32, + numActualFrames: 2, + numSkippableFrames: 1, + decompUnavailable: 0, + usesCheck: 1, + checksum: [1, 2, 3, 4], + nbFiles: 1, + dictID: 7, + }; + let expected_projection = project_list_file_info(&info); + let display_level = 3; + + for (status, expected_action, expected_return, publishes_info) in [ + ( + FIO_LIST_INFO_SUCCESS, + FIO_RUST_LIST_FILE_ACTION_SUCCESS, + 0, + true, + ), + ( + FIO_LIST_INFO_FRAME_ERROR, + FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR, + 1, + true, + ), + ( + FIO_LIST_INFO_NOT_ZSTD, + FIO_RUST_LIST_FILE_ACTION_NOT_ZSTD, + 1, + false, + ), + ( + FIO_LIST_INFO_FILE_ERROR, + FIO_RUST_LIST_FILE_ACTION_FILE_ERROR, + 1, + false, + ), + ( + FIO_LIST_INFO_TRUNCATED_INPUT, + FIO_RUST_LIST_FILE_ACTION_TRUNCATED_INPUT, + 1, + false, + ), + ] { + let mut state = ListFileTestState { + status, + info, + ..ListFileTestState::default() + }; + let callbacks = list_file_callbacks(&mut state); + let sentinel = FIO_listFileInfoProjection_t { + decompressedSize: 9, + compressedSize: 8, + numActualFrames: 7, + numSkippableFrames: 6, + decompUnavailable: 5, + usesCheck: 4, + nbFiles: 3, + }; + let mut output = sentinel; + + let result = unsafe { + FIO_rust_listFile( + file_name.as_ptr(), + display_level, + &mut output, + &callbacks, + ) + }; + + assert_eq!(result, expected_return, "status {status}"); + assert_eq!( + state.events, + if publishes_info { + if expected_action == FIO_RUST_LIST_FILE_ACTION_FRAME_ERROR { + vec!["get-info", "display-status", "display-info"] + } else { + vec!["get-info", "display-info"] + } + } else { + vec!["get-info", "display-status"] + }, + "status {status} event order" + ); + assert_eq!( + state.status_actions, + if expected_action == FIO_RUST_LIST_FILE_ACTION_SUCCESS { + Vec::new() + } else { + vec![(expected_action, display_level)] + }, + "status {status} diagnostic action" + ); + assert_eq!( + state.displayed, + publishes_info.then_some((info, display_level)), + "status {status} display gate" + ); + assert_eq!( + output, + if publishes_info { + expected_projection + } else { + sentinel + }, + "status {status} projection gate" + ); + } + } + + #[test] + fn list_file_callback_bridge_matches_c_layout() { + let word = size_of::<*mut c_void>(); + let callback = size_of::>(); + + assert_eq!(offset_of!(FIO_listFileCallbacks_t, opaque), 0); + assert_eq!(offset_of!(FIO_listFileCallbacks_t, getInfo), word); + assert_eq!( + offset_of!(FIO_listFileCallbacks_t, displayStatus), + word + callback + ); + assert_eq!( + offset_of!(FIO_listFileCallbacks_t, displayInfo), + word + 2 * callback + ); + assert_eq!(size_of::(), word + 3 * callback); + } + #[test] fn file_info_layout_matches_private_c_record() { let word = size_of::();