From 631dc4a3614dfdf8937b62a8204dcd12c31edf7f Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sun, 19 Jul 2026 12:48:13 +0200 Subject: [PATCH] feat(compress): move C parameter update policy into Rust Move ZSTD_CCtx_setCParams's validation and ordered parameter-update policy into the Rust projection. C retains the private ZSTD_CCtx_setParameter callback while Rust preserves the exact check/window/chain/hash/search/min-match/target/strategy order and error propagation. Test Plan: - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo test --manifest-path rust/Cargo.toml set_cparams -- --test-threads=1 - ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings - ulimit -v 41943040; make -B -C programs -j1 zstd - git diff --cached --check --- lib/compress/zstd_compress.c | 45 +++++++-- rust/src/zstd_compress.rs | 175 +++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 10 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 338228ce2..fcd6730b4 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -196,6 +196,23 @@ typedef char ZSTD_rust_init_cstream_advanced_state_layout[ == 5 * sizeof(void*) && sizeof(ZSTD_rust_initCStreamAdvancedState) == 6 * sizeof(void*)) ? 1 : -1]; +typedef size_t (*ZSTD_rust_setCParamsCheckCParams_f)( + void* context, ZSTD_compressionParameters cParams); +typedef size_t (*ZSTD_rust_setCParamsSetParameter_f)( + void* context, int param, int value); +typedef struct { + void* callbackContext; + ZSTD_rust_setCParamsCheckCParams_f checkCParams; + ZSTD_rust_setCParamsSetParameter_f setParameter; +} ZSTD_rust_setCParamsState; +size_t ZSTD_rust_setCParams(const ZSTD_rust_setCParamsState* state, + ZSTD_compressionParameters cParams); +typedef char ZSTD_rust_set_cparams_state_layout[ + (offsetof(ZSTD_rust_setCParamsState, callbackContext) == 0 + && offsetof(ZSTD_rust_setCParamsState, checkCParams) == sizeof(void*) + && offsetof(ZSTD_rust_setCParamsState, setParameter) == 2 * sizeof(void*) + && sizeof(ZSTD_rust_setCParamsState) == 3 * sizeof(void*)) + ? 1 : -1]; ZSTD_frameProgression ZSTD_rust_frameProgression(U64 consumedSrcSize, size_t buffered, U64 producedCSize); @@ -1873,20 +1890,28 @@ size_t ZSTD_CCtx_setParametersUsingCCtxParams( return 0; } +static size_t ZSTD_rust_setCParams_checkCParams( + void* context, ZSTD_compressionParameters cParams) +{ + (void)context; + return ZSTD_checkCParams(cParams); +} + +static size_t ZSTD_rust_setCParams_setParameter( + void* context, int param, int value) +{ + return ZSTD_CCtx_setParameter((ZSTD_CCtx*)context, (ZSTD_cParameter)param, value); +} + size_t ZSTD_CCtx_setCParams(ZSTD_CCtx* cctx, ZSTD_compressionParameters cparams) { + ZSTD_rust_setCParamsState state; ZSTD_STATIC_ASSERT(sizeof(cparams) == 7 * 4 /* all params are listed below */); DEBUGLOG(4, "ZSTD_CCtx_setCParams"); - /* only update if all parameters are valid */ - FORWARD_IF_ERROR(ZSTD_checkCParams(cparams), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, (int)cparams.windowLog), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_chainLog, (int)cparams.chainLog), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_hashLog, (int)cparams.hashLog), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_searchLog, (int)cparams.searchLog), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, (int)cparams.minMatch), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_targetLength, (int)cparams.targetLength), ""); - FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_strategy, (int)cparams.strategy), ""); - return 0; + state.callbackContext = cctx; + state.checkCParams = ZSTD_rust_setCParams_checkCParams; + state.setParameter = ZSTD_rust_setCParams_setParameter; + return ZSTD_rust_setCParams(&state, cparams); } size_t ZSTD_CCtx_setFParams(ZSTD_CCtx* cctx, ZSTD_frameParameters fparams) diff --git a/rust/src/zstd_compress.rs b/rust/src/zstd_compress.rs index d23992c9e..5e04a6c68 100644 --- a/rust/src/zstd_compress.rs +++ b/rust/src/zstd_compress.rs @@ -113,6 +113,13 @@ const ZSTD_CHUNKSIZE_MAX: usize = u32::MAX as usize - ZSTD_CURRENT_MAX; const ZSTD_E_END: c_int = 2; const ZSTD_E_CONTINUE: c_int = 0; const ZSTD_E_FLUSH: c_int = 1; +const ZSTD_C_WINDOW_LOG: c_int = 101; +const ZSTD_C_HASH_LOG: c_int = 102; +const ZSTD_C_CHAIN_LOG: c_int = 103; +const ZSTD_C_SEARCH_LOG: c_int = 104; +const ZSTD_C_MIN_MATCH: c_int = 105; +const ZSTD_C_TARGET_LENGTH: c_int = 106; +const ZSTD_C_STRATEGY: c_int = 107; const ZSTD_CSTREAM_STAGE_LOAD: c_int = 1; const ZSTD_CSTREAM_STAGE_FLUSH: c_int = 2; const ZSTD_BSS_COMPRESS: c_int = 0; @@ -1209,6 +1216,57 @@ pub unsafe extern "C" fn ZSTD_rust_initCStreamAdvanced( 0 } +type SetCParamsCheckCParamsFn = + unsafe extern "C" fn(*mut c_void, ZSTD_compressionParameters) -> usize; +type SetCParamsSetParameterFn = unsafe extern "C" fn(*mut c_void, c_int, c_int) -> usize; + +/// Explicit projection for `ZSTD_CCtx_setCParams`. +#[repr(C)] +pub struct ZSTD_rust_setCParamsState { + callback_context: *mut c_void, + check_c_params: SetCParamsCheckCParamsFn, + set_parameter: SetCParamsSetParameterFn, +} + +const _: () = { + assert!(offset_of!(ZSTD_rust_setCParamsState, callback_context) == 0); + assert!(offset_of!(ZSTD_rust_setCParamsState, check_c_params) == size_of::()); + assert!(offset_of!(ZSTD_rust_setCParamsState, set_parameter) == 2 * size_of::()); + assert!(size_of::() == 3 * size_of::()); +}; + +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_setCParams( + state: *const ZSTD_rust_setCParamsState, + cparams: ZSTD_compressionParameters, +) -> usize { + if state.is_null() { + return ERROR(ZstdErrorCode::Generic); + } + let state = unsafe { &*state }; + let result = unsafe { (state.check_c_params)(state.callback_context, cparams) }; + if ERR_isError(result) { + return result; + } + + let parameters = [ + (ZSTD_C_WINDOW_LOG, cparams.windowLog as c_int), + (ZSTD_C_CHAIN_LOG, cparams.chainLog as c_int), + (ZSTD_C_HASH_LOG, cparams.hashLog as c_int), + (ZSTD_C_SEARCH_LOG, cparams.searchLog as c_int), + (ZSTD_C_MIN_MATCH, cparams.minMatch as c_int), + (ZSTD_C_TARGET_LENGTH, cparams.targetLength as c_int), + (ZSTD_C_STRATEGY, cparams.strategy), + ]; + for (param, value) in parameters { + let result = unsafe { (state.set_parameter)(state.callback_context, param, value) }; + if ERR_isError(result) { + return result; + } + } + 0 +} + type CompressStreamBlockFn = unsafe extern "C" fn(*mut c_void, *mut c_void, usize, *const c_void, usize) -> usize; type CompressStreamResetFn = unsafe extern "C" fn(*mut c_void) -> usize; @@ -9627,6 +9685,123 @@ mod tests { ); } + #[derive(Default)] + struct SetCParamsTestContext { + events: Vec<&'static str>, + parameter_calls: Vec<(c_int, c_int)>, + check_result: usize, + set_result: usize, + fail_at: usize, + } + + unsafe fn set_cparams_test_context(context: *mut c_void) -> &'static mut SetCParamsTestContext { + unsafe { &mut *context.cast::() } + } + + unsafe extern "C" fn set_cparams_test_check( + context: *mut c_void, + _cparams: ZSTD_compressionParameters, + ) -> usize { + let context = unsafe { set_cparams_test_context(context) }; + context.events.push("check"); + context.check_result + } + + unsafe extern "C" fn set_cparams_test_set_parameter( + context: *mut c_void, + param: c_int, + value: c_int, + ) -> usize { + let context = unsafe { set_cparams_test_context(context) }; + context.events.push("set"); + context.parameter_calls.push((param, value)); + if context.fail_at != 0 && context.parameter_calls.len() == context.fail_at { + context.set_result + } else { + 0 + } + } + + fn set_cparams_test_state(context: &mut SetCParamsTestContext) -> ZSTD_rust_setCParamsState { + ZSTD_rust_setCParamsState { + callback_context: (context as *mut SetCParamsTestContext).cast(), + check_c_params: set_cparams_test_check, + set_parameter: set_cparams_test_set_parameter, + } + } + + fn set_cparams_test_params() -> ZSTD_compressionParameters { + ZSTD_compressionParameters { + windowLog: 10, + chainLog: 11, + hashLog: 12, + searchLog: 13, + minMatch: 4, + targetLength: 5, + strategy: 6, + } + } + + #[test] + fn set_cparams_preserves_validation_and_parameter_order() { + let cparams = set_cparams_test_params(); + let mut context = SetCParamsTestContext::default(); + let state = set_cparams_test_state(&mut context); + + let result = unsafe { ZSTD_rust_setCParams(&state, cparams) }; + + assert_eq!(result, 0); + assert_eq!( + context.events, + ["check", "set", "set", "set", "set", "set", "set", "set"] + ); + assert_eq!( + context.parameter_calls, + [ + (ZSTD_C_WINDOW_LOG, 10), + (ZSTD_C_CHAIN_LOG, 11), + (ZSTD_C_HASH_LOG, 12), + (ZSTD_C_SEARCH_LOG, 13), + (ZSTD_C_MIN_MATCH, 4), + (ZSTD_C_TARGET_LENGTH, 5), + (ZSTD_C_STRATEGY, 6), + ] + ); + } + + #[test] + fn set_cparams_stops_before_updates_after_validation_error() { + let cparams = set_cparams_test_params(); + let mut context = SetCParamsTestContext { + check_result: ERROR(ZstdErrorCode::ParameterOutOfBound), + ..SetCParamsTestContext::default() + }; + let state = set_cparams_test_state(&mut context); + + let result = unsafe { ZSTD_rust_setCParams(&state, cparams) }; + + assert_eq!(result, ERROR(ZstdErrorCode::ParameterOutOfBound)); + assert_eq!(context.events, ["check"]); + assert!(context.parameter_calls.is_empty()); + } + + #[test] + fn set_cparams_propagates_parameter_error_after_prior_updates() { + let cparams = set_cparams_test_params(); + let mut context = SetCParamsTestContext { + set_result: ERROR(ZstdErrorCode::StageWrong), + fail_at: 4, + ..SetCParamsTestContext::default() + }; + let state = set_cparams_test_state(&mut context); + + let result = unsafe { ZSTD_rust_setCParams(&state, cparams) }; + + assert_eq!(result, ERROR(ZstdErrorCode::StageWrong)); + assert_eq!(context.events, ["check", "set", "set", "set", "set"]); + assert_eq!(context.parameter_calls.len(), 4); + } + #[test] fn pledged_src_size_writes_the_init_stage_value_plus_one() { let mut pledged_src_size_plus_one = 0;