From 2fbf0285b2a1ac16e18007889b9422d88dfbdffc Mon Sep 17 00:00:00 2001
From: Stella Lau <laus@fb.com>
Date: Thu, 24 Aug 2017 11:25:41 -0700
Subject: [PATCH] Fix interaction with ZSTD_setCCtxParameter() and cleanup

---
 lib/compress/zstd_compress.c   | 35 ++++------------------------------
 lib/compress/zstdmt_compress.c | 12 ++++--------
 2 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c
index 8bf01c905..f3f602230 100644
--- a/lib/compress/zstd_compress.c
+++ b/lib/compress/zstd_compress.c
@@ -198,18 +198,22 @@ size_t ZSTD_sizeof_CStream(const ZSTD_CStream* zcs)
 const seqStore_t* ZSTD_getSeqStore(const ZSTD_CCtx* ctx) { return &(ctx->seqStore); }
 
 /* older variant; will be deprecated */
+/* Both requested and applied params need to be set as this function can be
+ * called before/after ZSTD_parameters have been applied. */
 size_t ZSTD_setCCtxParameter(ZSTD_CCtx* cctx, ZSTD_CCtxParameter param, unsigned value)
 {
     switch(param)
     {
     case ZSTD_p_forceWindow :
         cctx->requestedParams.forceWindow = value>0;
+        cctx->appliedParams.forceWindow = value>0;
         cctx->loadedDictEnd = 0;
         return 0;
     ZSTD_STATIC_ASSERT(ZSTD_dm_auto==0);
     ZSTD_STATIC_ASSERT(ZSTD_dm_rawContent==1);
     case ZSTD_p_forceRawDict :
         cctx->requestedParams.dictMode = (ZSTD_dictMode_e)(value>0);
+        cctx->appliedParams.dictMode = (ZSTD_dictMode_e)(value>0);
         return 0;
     default: return ERROR(parameter_unsupported);
     }
@@ -491,35 +495,6 @@ size_t ZSTD_CCtxParam_setParameter(
     }
 }
 
-#if 0
-static void ZSTD_debugPrintCCtxParams(ZSTD_CCtx_params* params)
-{
-    DEBUGLOG(2, "======CCtxParams======");
-    DEBUGLOG(2, "cParams: %u %u %u %u %u %u %u",
-             params->cParams.windowLog,
-             params->cParams.chainLog,
-             params->cParams.hashLog,
-             params->cParams.searchLog,
-             params->cParams.searchLength,
-             params->cParams.targetLength,
-             params->cParams.strategy);
-    DEBUGLOG(2, "fParams: %u %u %u",
-             params->fParams.contentSizeFlag,
-             params->fParams.checksumFlag,
-             params->fParams.noDictIDFlag);
-    DEBUGLOG(2, "cLevel, forceWindow: %u %u",
-             params->compressionLevel,
-             params->forceWindow);
-    DEBUGLOG(2, "dictionary: %u %u",
-             params->dictMode,
-             params->dictContentByRef);
-    DEBUGLOG(2, "multithreading: %u %u %u",
-             params->nbThreads,
-             params->jobSize,
-             params->overlapSizeLog);
-}
-#endif
-
 /**
  * This function should be updated whenever ZSTD_CCtx_params is updated.
  * Parameters are copied manually before the dictionary is loaded.
@@ -3910,8 +3885,6 @@ size_t ZSTD_initCStream_internal(ZSTD_CStream* zcs,
             return ERROR(memory_allocation);
         }
         ZSTD_freeCDict(zcs->cdictLocal);
-
-        /* Is a CCtx_params version needed? */
         zcs->cdictLocal = ZSTD_createCDict_advanced(
                                             dict, dictSize,
                                             params.dictContentByRef, params.dictMode,
diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c
index 6661e0a73..f7f02cfbb 100644
--- a/lib/compress/zstdmt_compress.c
+++ b/lib/compress/zstdmt_compress.c
@@ -186,7 +186,7 @@ static void ZSTDMT_releaseBuffer(ZSTDMT_bufferPool* bufPool, buffer_t buf)
     ZSTD_free(buf.start, bufPool->cMem);
 }
 
-/* Sets parameterse relevant to the compression job, initializing others to
+/* Sets parameters relevant to the compression job, initializing others to
  * default values. Notably, nbThreads should probably be zero. */
 static ZSTD_CCtx_params ZSTDMT_makeJobCCtxParams(ZSTD_CCtx_params const params)
 {
@@ -347,12 +347,9 @@ void ZSTDMT_compressChunk(void* jobDescription)
               ZSTD_CCtxParam_setParameter(&jobParams, ZSTD_p_dictMode, 1);
           size_t const forceWindowError =
               ZSTD_CCtxParam_setParameter(&jobParams, ZSTD_p_forceMaxWindow, !job->firstChunk);
-          /* TODO: new/old api do not interact well here (or with
-           * ZSTD_setCCtxParameter).
-           * ZSTD_compressBegin_advanced copies params directly to
-           * appliedParams. ZSTD_CCtx_setParameter sets params in requested
-           * parameters. They should not be mixed -- parameters should be passed
-           * directly */
+          /* Note: ZSTD_setCCtxParameter() should not be used here.
+           * ZSTD_compressBegin_advanced_internal() copies the ZSTD_CCtx_params
+           * directly to appliedParams. */
           size_t const initError = ZSTD_compressBegin_advanced_internal(cctx, job->srcStart, job->dictSize, jobParams, job->fullFrameSize);
             if (ZSTD_isError(initError) || ZSTD_isError(dictModeError) ||
                 ZSTD_isError(forceWindowError)) { job->cSize = initError; goto _endJob; }
@@ -752,7 +749,6 @@ size_t ZSTDMT_initCStream_internal(
     if (dict) {
         DEBUGLOG(4,"cdictLocal: %08X", (U32)(size_t)zcs->cdictLocal);
         ZSTD_freeCDict(zcs->cdictLocal);
-        /* TODO: cctxParam version? Is this correct? */
         zcs->cdictLocal = ZSTD_createCDict_advanced(dict, dictSize,
                                                     0 /* byRef */, ZSTD_dm_auto, /* note : a loadPrefix becomes an internal CDict */
                                                     requestedParams.cParams, zcs->cMem);