From 07a2a33135a5f5fd99c7f69e7dc0147c6894d252 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 7 Mar 2023 15:15:40 -0800 Subject: [PATCH] Add ZSTD_set{C,F,}Params() helper functions * Add ZSTD_setFParams() and ZSTD_setParams() * Modify ZSTD_setCParams() to use ZSTD_setParameter() to avoid a second path setting parameters * Add unit tests * Update documentation to suggest using them to replace deprecated functions Fixes #3396. --- lib/compress/zstd_compress.c | 39 ++++++++--- lib/zstd.h | 32 +++++---- tests/fuzzer.c | 127 +++++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 20 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index dc70dfbd8..72108311a 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1178,16 +1178,39 @@ size_t ZSTD_CCtx_setParametersUsingCCtxParams( size_t ZSTD_CCtx_setCParams(ZSTD_CCtx* cctx, ZSTD_compressionParameters cparams) { + ZSTD_STATIC_ASSERT(sizeof(cparams) == 7 * 4 /* all params are listed below */); DEBUGLOG(4, "ZSTD_CCtx_setCParams"); - assert(cctx != NULL); - if (cctx->streamStage != zcss_init) { - /* All parameters in @cparams are allowed to be updated during MT compression. - * This must be signaled, so that MT compression picks up the changes */ - cctx->cParamsChanged = 1; - } - /* only update if parameters are valid */ + /* only update if all parameters are valid */ FORWARD_IF_ERROR(ZSTD_checkCParams(cparams), ""); - cctx->requestedParams.cParams = cparams; + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, cparams.windowLog), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_chainLog, cparams.chainLog), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_hashLog, cparams.hashLog), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_searchLog, cparams.searchLog), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, cparams.minMatch), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_targetLength, cparams.targetLength), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_strategy, cparams.strategy), ""); + return 0; +} + +size_t ZSTD_CCtx_setFParams(ZSTD_CCtx* cctx, ZSTD_frameParameters fparams) +{ + ZSTD_STATIC_ASSERT(sizeof(fparams) == 3 * 4 /* all params are listed below */); + DEBUGLOG(4, "ZSTD_CCtx_setFParams"); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_contentSizeFlag, fparams.contentSizeFlag != 0), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, fparams.checksumFlag != 0), ""); + FORWARD_IF_ERROR(ZSTD_CCtx_setParameter(cctx, ZSTD_c_dictIDFlag, fparams.noDictIDFlag == 0), ""); + return 0; +} + +size_t ZSTD_CCtx_setParams(ZSTD_CCtx* cctx, ZSTD_parameters params) +{ + DEBUGLOG(4, "ZSTD_CCtx_setParams"); + /* First check cParams, because we want to update all or none. */ + FORWARD_IF_ERROR(ZSTD_checkCParams(params.cParams), ""); + /* Next set fParams, because this could fail if the cctx isn't in init stage. */ + FORWARD_IF_ERROR(ZSTD_CCtx_setFParams(cctx, params.fParams), ""); + /* Finally set cParams, which should succeed. */ + FORWARD_IF_ERROR(ZSTD_CCtx_setCParams(cctx, params.cParams), ""); return 0; } diff --git a/lib/zstd.h b/lib/zstd.h index 6c0c8eecb..56c43624a 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1803,12 +1803,26 @@ ZSTDLIB_STATIC_API size_t ZSTD_checkCParams(ZSTD_compressionParameters params); ZSTDLIB_STATIC_API ZSTD_compressionParameters ZSTD_adjustCParams(ZSTD_compressionParameters cPar, unsigned long long srcSize, size_t dictSize); /*! ZSTD_CCtx_setCParams() : - * Set all parameters provided within @cparams into the working @cctx. + * Set all parameters provided within @p cparams into the working @p cctx. * Note : if modifying parameters during compression (MT mode only), * note that changes to the .windowLog parameter will be ignored. - * @return 0 on success, or an error code (can be checked with ZSTD_isError()) */ + * @return 0 on success, or an error code (can be checked with ZSTD_isError()). + * On failure, no parameters are updated. + */ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_setCParams(ZSTD_CCtx* cctx, ZSTD_compressionParameters cparams); +/*! ZSTD_CCtx_setFParams() : + * Set all parameters provided within @p fparams into the working @p cctx. + * @return 0 on success, or an error code (can be checked with ZSTD_isError()). + */ +ZSTDLIB_STATIC_API size_t ZSTD_CCtx_setFParams(ZSTD_CCtx* cctx, ZSTD_frameParameters fparams); + +/*! ZSTD_CCtx_setParams() : + * Set all parameters provided within @p params into the working @p cctx. + * @return 0 on success, or an error code (can be checked with ZSTD_isError()). + */ +ZSTDLIB_STATIC_API size_t ZSTD_CCtx_setParams(ZSTD_CCtx* cctx, ZSTD_parameters params); + /*! ZSTD_compress_advanced() : * Note : this function is now DEPRECATED. * It can be replaced by ZSTD_compress2(), in combination with ZSTD_CCtx_setParameter() and other parameter setters. @@ -2452,12 +2466,9 @@ size_t ZSTD_initCStream_usingDict(ZSTD_CStream* zcs, int compressionLevel); /*! ZSTD_initCStream_advanced() : - * This function is DEPRECATED, and is approximately equivalent to: + * This function is DEPRECATED, and is equivalent to: * ZSTD_CCtx_reset(zcs, ZSTD_reset_session_only); - * // Pseudocode: Set each zstd parameter and leave the rest as-is. - * for ((param, value) : params) { - * ZSTD_CCtx_setParameter(zcs, param, value); - * } + * ZSTD_CCtx_setParams(zcs, params); * ZSTD_CCtx_setPledgedSrcSize(zcs, pledgedSrcSize); * ZSTD_CCtx_loadDictionary(zcs, dict, dictSize); * @@ -2486,12 +2497,9 @@ ZSTDLIB_STATIC_API size_t ZSTD_initCStream_usingCDict(ZSTD_CStream* zcs, const ZSTD_CDict* cdict); /*! ZSTD_initCStream_usingCDict_advanced() : - * This function is DEPRECATED, and is approximately equivalent to: + * This function is DEPRECATED, and is equivalent to: * ZSTD_CCtx_reset(zcs, ZSTD_reset_session_only); - * // Pseudocode: Set each zstd frame parameter and leave the rest as-is. - * for ((fParam, value) : fParams) { - * ZSTD_CCtx_setParameter(zcs, fParam, value); - * } + * ZSTD_CCtx_setFParams(zcs, fParams); * ZSTD_CCtx_setPledgedSrcSize(zcs, pledgedSrcSize); * ZSTD_CCtx_refCDict(zcs, cdict); * diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 85fa38475..fa5f89aa6 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1650,6 +1650,133 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3d : ZSTD_CCtx_setCParams() : ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + int value; + ZSTD_compressionParameters cparams = ZSTD_getCParams(1, 0, 0); + cparams.strategy = -1; + /* Set invalid cParams == no change. */ + CHECK(ZSTD_isError(ZSTD_CCtx_setCParams(cctx, cparams))); + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_chainLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_hashLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_searchLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_minMatch, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_targetLength, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_strategy, &value)); + CHECK_EQ(value, 0); + + cparams = ZSTD_getCParams(12, 0, 0); + CHECK_Z(ZSTD_CCtx_setCParams(cctx, cparams)); + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); + CHECK_EQ(value, (int)cparams.windowLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_chainLog, &value)); + CHECK_EQ(value, (int)cparams.chainLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_hashLog, &value)); + CHECK_EQ(value, (int)cparams.hashLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_searchLog, &value)); + CHECK_EQ(value, (int)cparams.searchLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_minMatch, &value)); + CHECK_EQ(value, (int)cparams.minMatch); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_targetLength, &value)); + CHECK_EQ(value, (int)cparams.targetLength); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_strategy, &value)); + CHECK_EQ(value, (int)cparams.strategy); + + ZSTD_freeCCtx(cctx); + } + + DISPLAYLEVEL(3, "test%3d : ZSTD_CCtx_setFParams() : ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + int value; + ZSTD_frameParameters fparams = {0, 1, 1}; + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_contentSizeFlag, &value)); + CHECK_EQ(value, 1); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_checksumFlag, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_dictIDFlag, &value)); + CHECK_EQ(value, 1); + + CHECK_Z(ZSTD_CCtx_setFParams(cctx, fparams)); + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_contentSizeFlag, &value)); + CHECK_EQ(value, fparams.contentSizeFlag); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_checksumFlag, &value)); + CHECK_EQ(value, fparams.checksumFlag); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_dictIDFlag, &value)); + CHECK_EQ(value, !fparams.noDictIDFlag); + + ZSTD_freeCCtx(cctx); + } + + DISPLAYLEVEL(3, "test%3d : ZSTD_CCtx_setCarams() : ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + int value; + ZSTD_parameters params = ZSTD_getParams(1, 0, 0); + params.cParams.strategy = -1; + /* Set invalid params == no change. */ + CHECK(ZSTD_isError(ZSTD_CCtx_setParams(cctx, params))); + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_chainLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_hashLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_searchLog, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_minMatch, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_targetLength, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_strategy, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_contentSizeFlag, &value)); + CHECK_EQ(value, 1); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_checksumFlag, &value)); + CHECK_EQ(value, 0); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_dictIDFlag, &value)); + CHECK_EQ(value, 1); + + params = ZSTD_getParams(12, 0, 0); + params.fParams.contentSizeFlag = 0; + params.fParams.checksumFlag = 1; + params.fParams.noDictIDFlag = 1; + CHECK_Z(ZSTD_CCtx_setParams(cctx, params)); + + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); + CHECK_EQ(value, (int)params.cParams.windowLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_chainLog, &value)); + CHECK_EQ(value, (int)params.cParams.chainLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_hashLog, &value)); + CHECK_EQ(value, (int)params.cParams.hashLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_searchLog, &value)); + CHECK_EQ(value, (int)params.cParams.searchLog); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_minMatch, &value)); + CHECK_EQ(value, (int)params.cParams.minMatch); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_targetLength, &value)); + CHECK_EQ(value, (int)params.cParams.targetLength); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_strategy, &value)); + CHECK_EQ(value, (int)params.cParams.strategy); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_contentSizeFlag, &value)); + CHECK_EQ(value, params.fParams.contentSizeFlag); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_checksumFlag, &value)); + CHECK_EQ(value, params.fParams.checksumFlag); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_dictIDFlag, &value)); + CHECK_EQ(value, !params.fParams.noDictIDFlag); + + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "test%3d : ldm conditionally enabled by default doesn't change cctx params: ", testNb++); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_outBuffer out = {NULL, 0, 0};