From 0175fdb408e37b436348d9bd56f432da090326a7 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 20 Jul 2026 18:00:09 +0200 Subject: [PATCH] refactor(cdict): move static initializer error policy to Rust Keep the private static-CDict initializer in C, but return its native size_t status through the callback boundary. Rust now owns the public success-or-NULL mapping, preserving the original workspace validation, callback order, and private layout ownership while making initializer failures explicit and unit-testable. Test Plan: ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings; cargo +nightly fmt --manifest-path rust/Cargo.toml --all -- --check; GCC and Clang syntax-only checks; make -j1 -C tests test (all 41 shell tests, fuzzer, zstd tester, and zstream tester passed). --- lib/compress/zstd_compress.c | 10 +++--- rust/src/zstd_compress_dictionary.rs | 53 +++++++++++++++++----------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 81d8ae330..f42148342 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -463,7 +463,7 @@ typedef void (*ZSTD_rust_initStaticCDictMoveWorkspace_f)( void* context, void* cdict); typedef void (*ZSTD_rust_initStaticCDictInitialize_f)( void* context, void* cdict, int useRowMatchFinder); -typedef void* (*ZSTD_rust_initStaticCDictInit_f)( +typedef size_t (*ZSTD_rust_initStaticCDictInit_f)( void* context, void* cdict); typedef struct { void* callbackContext; @@ -7306,16 +7306,14 @@ static void ZSTD_rust_initStaticCDict_initialize( dictionary->compressionLevel = ZSTD_NO_CLEVEL; } -static void* ZSTD_rust_initStaticCDict_init(void* opaque, void* cdict) +static size_t ZSTD_rust_initStaticCDict_init(void* opaque, void* cdict) { ZSTD_rust_initStaticCDictContext const* const context = (const ZSTD_rust_initStaticCDictContext*)opaque; - if (ZSTD_isError( ZSTD_initCDict_internal( + return ZSTD_initCDict_internal( (ZSTD_CDict*)cdict, context->dict, context->dictSize, context->dictLoadMethod, context->dictContentType, - context->params) )) - return NULL; - return cdict; + context->params); } const ZSTD_CDict* ZSTD_initStaticCDict( diff --git a/rust/src/zstd_compress_dictionary.rs b/rust/src/zstd_compress_dictionary.rs index e71bd9951..9b1d58643 100644 --- a/rust/src/zstd_compress_dictionary.rs +++ b/rust/src/zstd_compress_dictionary.rs @@ -1091,7 +1091,7 @@ type InitStaticCDictCreateWorkspaceFn = unsafe extern "C" fn(*mut c_void); type InitStaticCDictReserveObjectFn = unsafe extern "C" fn(*mut c_void) -> *mut c_void; type InitStaticCDictMoveWorkspaceFn = unsafe extern "C" fn(*mut c_void, *mut c_void); type InitStaticCDictInitializeFn = unsafe extern "C" fn(*mut c_void, *mut c_void, c_int); -type InitStaticCDictFn = unsafe extern "C" fn(*mut c_void, *mut c_void) -> *mut c_void; +type InitStaticCDictFn = unsafe extern "C" fn(*mut c_void, *mut c_void) -> usize; /// Explicit projection for the public `ZSTD_initStaticCDict` wrapper. /// @@ -1130,7 +1130,16 @@ const _: () = { assert!(size_of::() == size_of::<[usize; 12]>()); }; +#[inline] +fn static_cdict_init_succeeded(result: usize) -> bool { + !ERR_isError(result) +} + /// Validate static CDict workspace ownership before entering the C leaf. +/// +/// The C initializer returns the private `ZSTD_initCDict_internal()` result; +/// Rust owns the public pointer-or-NULL mapping so the C callback remains a +/// layout/configuration operation only. #[no_mangle] pub unsafe extern "C" fn ZSTD_rust_initStaticCDict( state: *const ZSTD_rust_initStaticCDictState, @@ -1170,10 +1179,15 @@ pub unsafe extern "C" fn ZSTD_rust_initStaticCDict( if cdict.is_null() { return ptr::null_mut(); } - unsafe { + let init_result = unsafe { (state.move_workspace)(state.callback_context, cdict); (state.initialize)(state.callback_context, cdict, use_row_match_finder); (state.init)(state.callback_context, cdict) + }; + if static_cdict_init_succeeded(init_result) { + cdict + } else { + ptr::null_mut() } } @@ -5205,7 +5219,7 @@ mod tests { init_cdict: *mut c_void, use_row_match_finder: c_int, initialized: bool, - result: *mut c_void, + init_result: usize, } unsafe extern "C" fn init_static_cdict_test_create_workspace(context: *mut c_void) { @@ -5245,11 +5259,11 @@ mod tests { unsafe extern "C" fn init_static_cdict_test_init( context: *mut c_void, cdict: *mut c_void, - ) -> *mut c_void { + ) -> usize { let probe = unsafe { &mut *context.cast::() }; probe.events.push("init"); probe.init_cdict = cdict; - probe.result + probe.init_result } fn init_static_cdict_test_state( @@ -5308,12 +5322,18 @@ mod tests { } } + #[test] + fn static_cdict_initializer_maps_error_status_to_null() { + assert!(static_cdict_init_succeeded(0)); + assert!(static_cdict_init_succeeded(1)); + assert!(!static_cdict_init_succeeded(ERROR( + ZstdErrorCode::MemoryAllocation + ))); + } + #[test] fn init_static_cdict_rejects_an_unaligned_workspace_before_callback() { - let mut probe = InitStaticCDictProbe { - result: ptr::dangling_mut(), - ..Default::default() - }; + let mut probe = InitStaticCDictProbe::default(); let mut workspace = [0usize; 2]; let unaligned_workspace = workspace.as_mut_ptr().cast::().wrapping_add(1).cast(); let c_params = init_static_cdict_test_cparams(); @@ -5335,10 +5355,7 @@ mod tests { #[test] fn init_static_cdict_rejects_insufficient_workspace_before_callback() { - let mut probe = InitStaticCDictProbe { - result: ptr::dangling_mut(), - ..Default::default() - }; + let mut probe = InitStaticCDictProbe::default(); let mut workspace = [0usize; 1]; let c_params = init_static_cdict_test_cparams(); let state = init_static_cdict_test_state( @@ -5362,7 +5379,6 @@ mod tests { let cdict = ptr::dangling_mut::(); let mut probe = InitStaticCDictProbe { reserved_cdict: cdict, - result: ptr::dangling_mut(), ..Default::default() }; let c_params = init_static_cdict_test_cparams(); @@ -5379,7 +5395,7 @@ mod tests { let result = unsafe { ZSTD_rust_initStaticCDict(&state) }; - assert_eq!(result, probe.result); + assert_eq!(result, cdict); assert_eq!( probe.events, [ @@ -5428,7 +5444,7 @@ mod tests { let cdict = ptr::dangling_mut::(); let mut probe = InitStaticCDictProbe { reserved_cdict: cdict, - result: ptr::null_mut(), + init_result: ERROR(ZstdErrorCode::MemoryAllocation), ..Default::default() }; let c_params = init_static_cdict_test_cparams(); @@ -5462,10 +5478,7 @@ mod tests { #[test] fn init_static_cdict_rejects_a_null_workspace() { - let mut probe = InitStaticCDictProbe { - result: ptr::dangling_mut(), - ..Default::default() - }; + let mut probe = InitStaticCDictProbe::default(); let c_params = init_static_cdict_test_cparams(); let state = init_static_cdict_test_state( &mut probe,