From 3feb9370c81ff6fa8a121789b22681d33aefa2d4 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 18 Jul 2026 03:01:08 +0200 Subject: [PATCH] feat(compress): move CCtx parameter allocation to Rust Move CCtx_params allocation, initialization, and release behind Rust ABI helpers while preserving the public C wrappers and custom allocator contract. Keep parameter bounds and policy in the existing C implementation. Test Plan:\n- cargo clippy --manifest-path rust/Cargo.toml\n- cargo clippy --manifest-path rust/Cargo.toml --benches\n- cargo clippy --manifest-path rust/Cargo.toml --tests\n- cargo +nightly fmt --manifest-path rust/Cargo.toml --all\n- cargo test --manifest-path rust/Cargo.toml --no-default-features --features compression\n- make -B -C lib -j2 lib\n- make -B -C tests -j2 test-rust-lib-smoke\n- make -B -C tests -j2 test-zstream\n- make -B -C tests -j2 test-cli-tests --- lib/compress/zstd_compress.c | 15 +--- rust/src/zstd_compress_params_api.rs | 116 +++++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 16 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index ca5389f0b..cad72dc4f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -87,6 +87,8 @@ size_t ZSTD_rust_cctx_params_clamp_bounds(int param, int* value); int ZSTD_rust_cctx_params_within_bounds(int param, int value); int ZSTD_rust_cctx_params_is_multithreaded(void); int ZSTD_rust_cctx_params_job_size_min(void); +ZSTD_CCtx_params* ZSTD_rust_createCCtxParams(ZSTD_customMem customMem); +size_t ZSTD_rust_freeCCtxParams(ZSTD_CCtx_params* params); #define ZSTD_RUST_CCTX_PARAMS_ASSERT(name, condition) \ typedef char name[(condition) ? 1 : -1] @@ -531,14 +533,7 @@ static ZSTD_CCtx_params ZSTD_makeCCtxParamsFromCParams( static ZSTD_CCtx_params* ZSTD_createCCtxParams_advanced( ZSTD_customMem customMem) { - ZSTD_CCtx_params* params; - if ((!customMem.customAlloc) ^ (!customMem.customFree)) return NULL; - params = (ZSTD_CCtx_params*)ZSTD_customCalloc( - sizeof(ZSTD_CCtx_params), customMem); - if (!params) { return NULL; } - ZSTD_CCtxParams_init(params, ZSTD_CLEVEL_DEFAULT); - params->customMem = customMem; - return params; + return ZSTD_rust_createCCtxParams(customMem); } ZSTD_CCtx_params* ZSTD_createCCtxParams(void) @@ -548,9 +543,7 @@ ZSTD_CCtx_params* ZSTD_createCCtxParams(void) size_t ZSTD_freeCCtxParams(ZSTD_CCtx_params* params) { - if (params == NULL) { return 0; } - ZSTD_customFree(params, params->customMem); - return 0; + return ZSTD_rust_freeCCtxParams(params); } #define ZSTD_NO_CLEVEL 0 diff --git a/rust/src/zstd_compress_params_api.rs b/rust/src/zstd_compress_params_api.rs index d03f790cc..44517edca 100644 --- a/rust/src/zstd_compress_params_api.rs +++ b/rust/src/zstd_compress_params_api.rs @@ -5,10 +5,10 @@ //! Rust implementation of the narrow `ZSTD_CCtx_params` object API. //! -//! Allocation, advanced initialization, bounds calculation, and context -//! policy remain in `zstd_compress.c`. This module owns only the object -//! reset/initialization, parameter set/get, and sequence-producer registration -//! entry points. +//! Advanced initialization, bounds calculation, and context policy remain in +//! `zstd_compress.c`. This module owns the parameter object's storage +//! lifecycle, reset/initialization, parameter set/get, and sequence-producer +//! registration entry points. use crate::errors::{ZstdErrorCode, ERROR}; #[cfg(test)] @@ -24,7 +24,7 @@ type ZstdFreeFunction = unsafe extern "C" fn(*mut c_void, *mut c_void); /// ABI-compatible representation of `ZSTD_customMem`. #[repr(C)] #[derive(Clone, Copy)] -struct ZSTD_customMem { +pub struct ZSTD_customMem { customAlloc: Option, customFree: Option, opaque: *mut c_void, @@ -301,6 +301,61 @@ unsafe fn init_impl(params: *mut ZSTD_CCtx_params, compression_level: c_int) -> 0 } +unsafe fn custom_calloc(size: usize, custom_mem: ZSTD_customMem) -> *mut c_void { + if let Some(custom_alloc) = custom_mem.customAlloc { + let allocation = unsafe { custom_alloc(custom_mem.opaque, size) }; + if !allocation.is_null() { + unsafe { ptr::write_bytes(allocation.cast::(), 0, size) }; + } + allocation + } else { + unsafe { libc::calloc(1, size) } + } +} + +unsafe fn custom_free(allocation: *mut c_void, custom_mem: ZSTD_customMem) { + if allocation.is_null() { + return; + } + if let Some(custom_free) = custom_mem.customFree { + unsafe { custom_free(custom_mem.opaque, allocation) }; + } else { + unsafe { libc::free(allocation) }; + } +} + +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_createCCtxParams( + custom_mem: ZSTD_customMem, +) -> *mut ZSTD_CCtx_params { + if custom_mem.customAlloc.is_some() != custom_mem.customFree.is_some() { + return ptr::null_mut(); + } + + let params = unsafe { custom_calloc(size_of::(), custom_mem) } + .cast::(); + if params.is_null() { + return ptr::null_mut(); + } + + if unsafe { ZSTD_CCtxParams_init(params, DEFAULT_CLEVEL) } != 0 { + unsafe { custom_free(params.cast(), custom_mem) }; + return ptr::null_mut(); + } + unsafe { (*params).customMem = custom_mem }; + params +} + +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_freeCCtxParams(params: *mut ZSTD_CCtx_params) -> usize { + if params.is_null() { + return 0; + } + let custom_mem = unsafe { (*params).customMem }; + unsafe { custom_free(params.cast(), custom_mem) }; + 0 +} + #[no_mangle] pub unsafe extern "C" fn ZSTD_CCtxParams_reset(params: *mut ZSTD_CCtx_params) -> usize { unsafe { init_impl(params, DEFAULT_CLEVEL) } @@ -750,6 +805,7 @@ mod tests { use super::*; use crate::errors::{ERR_getErrorCode, ERR_isError}; use std::mem::{align_of, offset_of, size_of, MaybeUninit}; + use std::sync::atomic::{AtomicUsize, Ordering}; #[test] fn private_parameter_mirror_matches_the_c_layout() { @@ -873,6 +929,56 @@ mod tests { ); } + #[test] + fn allocation_lifecycle_preserves_default_and_custom_memory_contracts() { + unsafe extern "C" fn counting_alloc(opaque: *mut c_void, size: usize) -> *mut c_void { + unsafe { + (*opaque.cast::()).fetch_add(1, Ordering::Relaxed); + libc::malloc(size) + } + } + + unsafe extern "C" fn counting_free(opaque: *mut c_void, allocation: *mut c_void) { + unsafe { + (*opaque.cast::()).fetch_add(1, Ordering::Relaxed); + libc::free(allocation); + } + } + + unsafe { + let params = ZSTD_rust_createCCtxParams(ZSTD_customMem { + customAlloc: None, + customFree: None, + opaque: ptr::null_mut(), + }); + assert!(!params.is_null()); + assert_eq!((*params).compressionLevel, DEFAULT_CLEVEL); + assert_eq!((*params).fParams.contentSizeFlag, 1); + assert_eq!(ZSTD_rust_freeCCtxParams(params), 0); + assert_eq!(ZSTD_rust_freeCCtxParams(ptr::null_mut()), 0); + + let calls = AtomicUsize::new(0); + let custom_mem = ZSTD_customMem { + customAlloc: Some(counting_alloc), + customFree: Some(counting_free), + opaque: (&calls as *const AtomicUsize).cast_mut().cast(), + }; + let params = ZSTD_rust_createCCtxParams(custom_mem); + assert!(!params.is_null()); + assert_eq!((*params).customMem.opaque, custom_mem.opaque); + assert_eq!(calls.load(Ordering::Relaxed), 1); + assert_eq!(ZSTD_rust_freeCCtxParams(params), 0); + assert_eq!(calls.load(Ordering::Relaxed), 2); + + assert!(ZSTD_rust_createCCtxParams(ZSTD_customMem { + customAlloc: Some(counting_alloc), + customFree: None, + opaque: ptr::null_mut(), + }) + .is_null()); + } + } + #[test] fn init_reset_and_set_get_preserve_public_semantics() { let mut storage = MaybeUninit::::uninit();