[libzstd] Clean up parameter code

* Move all ZSTDMT parameter setting code to ZSTD_CCtxParams_*Parameter().
  ZSTDMT now calls these functions, so we can keep all the logic in the
  same place.
* Clean up `ZSTD_CCtx_setParameter()` to only add extra checks where needed.
* Clean up `ZSTDMT_initJobCCtxParams()` by copying all parameters by default,
  and then zeroing the ones that need to be zeroed. We've missed adding several
  parameters here, and it makes more sense to only have to update it if you
  change something in ZSTDMT.
* Add `ZSTDMT_cParam_clampBounds()` to clamp a parameter into its valid
  range. Use this to keep backwards compatibility when setting ZSTDMT parameters,
  which clamp into the valid range.

lib/compress/zstd_compress.c

@@ -385,6 +385,18 @@ static int ZSTD_cParam_withinBounds(ZSTD_cParameter cParam, int value)
     return 1;
 }
 
+/* ZSTD_cParam_clampBounds:
+ * Clamps the value into the bounded range.
+ */
+static size_t ZSTD_cParam_clampBounds(ZSTD_cParameter cParam, int* value)
+{
+    ZSTD_bounds const bounds = ZSTD_cParam_getBounds(cParam);
+    if (ZSTD_isError(bounds.error)) return bounds.error;
+    if (*value < bounds.lowerBound) *value = bounds.lowerBound;
+    if (*value > bounds.upperBound) *value = bounds.upperBound;
+    return 0;
+}
+
 #define BOUNDCHECK(cParam, val) { \
     RETURN_ERROR_IF(!ZSTD_cParam_withinBounds(cParam,val), \
                     parameter_outOfBound); \
@@ -438,13 +450,10 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, int value)
 
     switch(param)
     {
-    case ZSTD_c_format :
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
-
     case ZSTD_c_compressionLevel:
         RETURN_ERROR_IF(cctx->cdict, stage_wrong,
                         "compression level is configured in cdict");
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
+        break;
 
     case ZSTD_c_windowLog:
     case ZSTD_c_hashLog:
@@ -455,44 +464,37 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, int value)
     case ZSTD_c_strategy:
         RETURN_ERROR_IF(cctx->cdict, stage_wrong,
                         "cparams are configured in cdict");
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
+        break;
-
-    case ZSTD_c_contentSizeFlag:
-    case ZSTD_c_checksumFlag:
-    case ZSTD_c_dictIDFlag:
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
-
-    case ZSTD_c_forceMaxWindow :  /* Force back-references to remain < windowSize,
-                                   * even when referencing into Dictionary content.
-                                   * default : 0 when using a CDict, 1 when using a Prefix */
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
-
-    case ZSTD_c_forceAttachDict:
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
-
-    case ZSTD_c_literalCompressionMode:
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
 
     case ZSTD_c_nbWorkers:
         RETURN_ERROR_IF((value!=0) && cctx->staticSize, parameter_unsupported,
                         "MT not compatible with static alloc");
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
+        break;
 
+    case ZSTD_c_ldmHashRateLog:
+        RETURN_ERROR_IF(cctx->cdict, stage_wrong,
+                        "LDM hash rate log is configured in cdict");
+        break;
+
+    case ZSTD_c_format:
+    case ZSTD_c_contentSizeFlag:
+    case ZSTD_c_checksumFlag:
+    case ZSTD_c_dictIDFlag:
+    case ZSTD_c_forceMaxWindow:
+    case ZSTD_c_forceAttachDict:
+    case ZSTD_c_literalCompressionMode:
     case ZSTD_c_jobSize:
     case ZSTD_c_overlapLog:
     case ZSTD_c_rsyncable:
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
-
     case ZSTD_c_enableLongDistanceMatching:
     case ZSTD_c_ldmHashLog:
     case ZSTD_c_ldmMinMatch:
     case ZSTD_c_ldmBucketSizeLog:
-    case ZSTD_c_ldmHashRateLog:
+        break;
-        RETURN_ERROR_IF(cctx->cdict, stage_wrong);
-        return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
 
     default: RETURN_ERROR(parameter_unsupported);
     }
+    return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value);
 }
 
 size_t ZSTD_CCtxParam_setParameter(ZSTD_CCtx_params* CCtxParams,
@@ -507,11 +509,9 @@ size_t ZSTD_CCtxParam_setParameter(ZSTD_CCtx_params* CCtxParams,
         return (size_t)CCtxParams->format;
 
     case ZSTD_c_compressionLevel : {
-        int cLevel = value;
+        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(param, &value));
-        if (cLevel > ZSTD_maxCLevel()) cLevel = ZSTD_maxCLevel();
+        if (value) {  /* 0 : does not change current level */
-        if (cLevel < ZSTD_minCLevel()) cLevel = ZSTD_minCLevel();
+            CCtxParams->compressionLevel = value;
-        if (cLevel) {  /* 0 : does not change current level */
-            CCtxParams->compressionLevel = cLevel;
         }
         if (CCtxParams->compressionLevel >= 0) return CCtxParams->compressionLevel;
         return 0;  /* return type (size_t) cannot represent negative values */
@@ -597,28 +597,43 @@ size_t ZSTD_CCtxParam_setParameter(ZSTD_CCtx_params* CCtxParams,
         RETURN_ERROR_IF(value!=0, parameter_unsupported, "not compiled with multithreading");
         return 0;
 #else
-        return ZSTDMT_CCtxParam_setNbWorkers(CCtxParams, value);
+        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(param, &value));
+        CCtxParams->nbWorkers = value;
+        return CCtxParams->nbWorkers;
 #endif
 
     case ZSTD_c_jobSize :
 #ifndef ZSTD_MULTITHREAD
-        RETURN_ERROR(parameter_unsupported, "not compiled with multithreading");
+        RETURN_ERROR_IF(value!=0, parameter_unsupported, "not compiled with multithreading");
+        return 0;
 #else
-        return ZSTDMT_CCtxParam_setMTCtxParameter(CCtxParams, ZSTDMT_p_jobSize, value);
+        /* Adjust to the minimum non-default value. */
+        if (value != 0 && value < ZSTDMT_JOBSIZE_MIN)
+            value = ZSTDMT_JOBSIZE_MIN;
+        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(param, &value));
+        assert(value >= 0);
+        CCtxParams->jobSize = value;
+        return CCtxParams->jobSize;
 #endif
 
     case ZSTD_c_overlapLog :
 #ifndef ZSTD_MULTITHREAD
-        RETURN_ERROR(parameter_unsupported, "not compiled with multithreading");
+        RETURN_ERROR_IF(value!=0, parameter_unsupported, "not compiled with multithreading");
+        return 0;
 #else
-        return ZSTDMT_CCtxParam_setMTCtxParameter(CCtxParams, ZSTDMT_p_overlapLog, value);
+        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(ZSTD_c_overlapLog, &value));
+        CCtxParams->overlapLog = value;
+        return CCtxParams->overlapLog;
 #endif
 
     case ZSTD_c_rsyncable :
 #ifndef ZSTD_MULTITHREAD
-        RETURN_ERROR(parameter_unsupported, "not compiled with multithreading");
+        RETURN_ERROR_IF(value!=0, parameter_unsupported, "not compiled with multithreading");
+        return 0;
 #else
-        return ZSTDMT_CCtxParam_setMTCtxParameter(CCtxParams, ZSTDMT_p_rsyncable, value);
+        FORWARD_IF_ERROR(ZSTD_cParam_clampBounds(ZSTD_c_overlapLog, &value));
+        CCtxParams->rsyncable = value;
+        return CCtxParams->rsyncable;
 #endif
 
     case ZSTD_c_enableLongDistanceMatching :

lib/compress/zstdmt_compress.c

@@ -864,11 +864,7 @@ static size_t ZSTDMT_expandJobsTable (ZSTDMT_CCtx* mtctx, U32 nbWorkers) {
  * Internal use only */
 size_t ZSTDMT_CCtxParam_setNbWorkers(ZSTD_CCtx_params* params, unsigned nbWorkers)
 {
-    if (nbWorkers > ZSTDMT_NBWORKERS_MAX) nbWorkers = ZSTDMT_NBWORKERS_MAX;
+    return ZSTD_CCtxParam_setParameter(params, ZSTD_c_nbWorkers, (int)nbWorkers);
-    params->nbWorkers = nbWorkers;
-    params->overlapLog = ZSTDMT_OVERLAPLOG_DEFAULT;
-    params->jobSize = 0;
-    return nbWorkers;
 }
 
 ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_customMem cMem)
@@ -986,26 +982,13 @@ ZSTDMT_CCtxParam_setMTCtxParameter(ZSTD_CCtx_params* params,
     {
     case ZSTDMT_p_jobSize :
         DEBUGLOG(4, "ZSTDMT_CCtxParam_setMTCtxParameter : set jobSize to %i", value);
-        if ( value != 0  /* default */
+        return ZSTD_CCtxParam_setParameter(params, ZSTD_c_jobSize, value);
-          && value < ZSTDMT_JOBSIZE_MIN)
-            value = ZSTDMT_JOBSIZE_MIN;
-        assert(value >= 0);
-        if (value > ZSTDMT_JOBSIZE_MAX) value = ZSTDMT_JOBSIZE_MAX;
-        params->jobSize = value;
-        return value;
-
     case ZSTDMT_p_overlapLog :
         DEBUGLOG(4, "ZSTDMT_p_overlapLog : %i", value);
-        if (value < ZSTD_OVERLAPLOG_MIN) value = ZSTD_OVERLAPLOG_MIN;
+        return ZSTD_CCtxParam_setParameter(params, ZSTD_c_overlapLog, value);
-        if (value > ZSTD_OVERLAPLOG_MAX) value = ZSTD_OVERLAPLOG_MAX;
-        params->overlapLog = value;
-        return value;
-
     case ZSTDMT_p_rsyncable :
-        value = (value != 0);
+        DEBUGLOG(4, "ZSTD_p_rsyncable : %i", value);
-        params->rsyncable = value;
+        return ZSTD_CCtxParam_setParameter(params, ZSTD_c_rsyncable, value);
-        return value;
-
     default :
         return ERROR(parameter_unsupported);
     }
@@ -1021,32 +1004,29 @@ size_t ZSTDMT_getMTCtxParameter(ZSTDMT_CCtx* mtctx, ZSTDMT_parameter parameter,
 {
     switch (parameter) {
     case ZSTDMT_p_jobSize:
-        assert(mtctx->params.jobSize <= INT_MAX);
+        return ZSTD_CCtxParam_getParameter(&mtctx->params, ZSTD_c_jobSize, value);
-        *value = (int)(mtctx->params.jobSize);
-        break;
     case ZSTDMT_p_overlapLog:
-        *value = mtctx->params.overlapLog;
+        return ZSTD_CCtxParam_getParameter(&mtctx->params, ZSTD_c_overlapLog, value);
-        break;
     case ZSTDMT_p_rsyncable:
-        *value = mtctx->params.rsyncable;
+        return ZSTD_CCtxParam_getParameter(&mtctx->params, ZSTD_c_rsyncable, value);
-        break;
     default:
         return ERROR(parameter_unsupported);
     }
-    return 0;
 }
 
 /* Sets parameters relevant to the compression job,
  * initializing others to default values. */
 static ZSTD_CCtx_params ZSTDMT_initJobCCtxParams(ZSTD_CCtx_params const params)
 {
-    ZSTD_CCtx_params jobParams;
+    ZSTD_CCtx_params jobParams = params;
-    memset(&jobParams, 0, sizeof(jobParams));
+    /* Clear parameters related to multithreading */
-
+    jobParams.forceWindow = 0;
-    jobParams.cParams = params.cParams;
+    jobParams.nbWorkers = 0;
-    jobParams.fParams = params.fParams;
+    jobParams.jobSize = 0;
-    jobParams.compressionLevel = params.compressionLevel;
+    jobParams.overlapLog = 0;
-
+    jobParams.rsyncable = 0;
+    memset(&jobParams.ldmParams, 0, sizeof(ldmParams_t));
+    memset(&jobParams.customMem, 0, sizeof(ZSTD_customMem));
     return jobParams;
 }
 

tests/regression/config.c

@@ -90,6 +90,17 @@ static config_t mt_ldm = {
     .param_values = PARAM_VALUES(mt_ldm_param_values),
 };
 
+static param_value_t mt_advanced_param_values[] = {
+    {.param = ZSTD_c_nbWorkers, .value = 2},
+    {.param = ZSTD_c_literalCompressionMode, .value = ZSTD_lcm_uncompressed},
+};
+
+static config_t mt_advanced = {
+    .name = "multithreaded with advanced params",
+    .cli_args = "-T2 --no-compressed-literals",
+    .param_values = PARAM_VALUES(mt_advanced_param_values),
+};
+
 static param_value_t const small_wlog_param_values[] = {
     {.param = ZSTD_c_windowLog, .value = 10},
 };
@@ -191,6 +202,7 @@ static config_t const* g_configs[] = {
     &uncompressed_literals,
     &uncompressed_literals_opt,
     &huffman_literals,
+    &mt_advanced,
     NULL,
 };