From b8136f019a7dc50b51fc51e68cfd38bace101057 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 27 May 2017 00:03:08 -0700 Subject: [PATCH 1/3] static dctx is incompatible with legacy support documented, and runtime tested --- doc/zstd_manual.html | 3 ++- lib/decompress/zstd_decompress.c | 9 ++++++++- lib/zstd.h | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/zstd_manual.html b/doc/zstd_manual.html index a4b59bc4..156e7dd9 100644 --- a/doc/zstd_manual.html +++ b/doc/zstd_manual.html @@ -718,7 +718,8 @@ size_t ZSTD_CDict_loadDictionary(ZSTD_CDict* cdict, const void* dict, size_t dic @return : pointer to ZSTD_DCtx*, or NULL if error (size too small) Note : zstd will never resize nor malloc() when using a static dctx. If it needs more memory than available, it will simply error out. - Note 2 : there is no corresponding "free" function. + Note 2 : static dctx is incompatible with legacy support + Note 3 : there is no corresponding "free" function. Since workspace was allocated externally, it must be freed externally. Limitation : currently not compatible with internal DDict creation, such as ZSTD_initDStream_usingDict(). diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index fe7446ab..a5972438 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1586,6 +1586,8 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, size_t decodedSize; size_t const frameSize = ZSTD_findFrameCompressedSizeLegacy(src, srcSize); if (ZSTD_isError(frameSize)) return frameSize; + /* legacy support is incompatible with static dctx */ + if (dctx->staticSize) return ERROR(memory_allocation); decodedSize = ZSTD_decompressLegacy(dst, dstCapacity, src, frameSize, dict, dictSize); @@ -2258,8 +2260,11 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB U32 someMoreWork = 1; #if defined(ZSTD_LEGACY_SUPPORT) && (ZSTD_LEGACY_SUPPORT>=1) - if (zds->legacyVersion) + if (zds->legacyVersion) { + /* legacy support is incompatible with static dctx */ + if (zds->staticSize) return ERROR(memory_allocation); return ZSTD_decompressLegacyStream(zds->legacyContext, zds->legacyVersion, output, input); + } #endif while (someMoreWork) { @@ -2277,6 +2282,8 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB if (legacyVersion) { const void* const dict = zds->ddict ? zds->ddict->dictContent : NULL; size_t const dictSize = zds->ddict ? zds->ddict->dictSize : 0; + /* legacy support is incompatible with static dctx */ + if (zds->staticSize) return ERROR(memory_allocation); CHECK_F(ZSTD_initLegacyStream(&zds->legacyContext, zds->previousLegacyVersion, legacyVersion, dict, dictSize)); zds->legacyVersion = zds->previousLegacyVersion = legacyVersion; diff --git a/lib/zstd.h b/lib/zstd.h index b1d8ecad..3bbdb76f 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -844,7 +844,8 @@ ZSTDLIB_API ZSTD_DCtx* ZSTD_createDCtx_advanced(ZSTD_customMem customMem); * @return : pointer to ZSTD_DCtx*, or NULL if error (size too small) * Note : zstd will never resize nor malloc() when using a static dctx. * If it needs more memory than available, it will simply error out. - * Note 2 : there is no corresponding "free" function. + * Note 2 : static dctx is incompatible with legacy support + * Note 3 : there is no corresponding "free" function. * Since workspace was allocated externally, it must be freed externally. * Limitation : currently not compatible with internal DDict creation, * such as ZSTD_initDStream_usingDict(). From e07115910137fda805c1ef14f7834ca7ba896677 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 27 May 2017 00:21:33 -0700 Subject: [PATCH 2/3] mtctx->jobs allocate its own memory space to make ZSTDMT_CCtx_s size predictable so that it can be included in CCtx --- lib/compress/zstdmt_compress.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index fc7f52a2..d41825e8 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -271,6 +271,7 @@ _endJob: struct ZSTDMT_CCtx_s { POOL_ctx* factory; + ZSTDMT_jobDescription* jobs; ZSTDMT_bufferPool* buffPool; ZSTDMT_CCtxPool* cctxPool; pthread_mutex_t jobCompleted_mutex; @@ -294,10 +295,9 @@ struct ZSTDMT_CCtx_s { size_t sectionSize; ZSTD_CDict* cdict; ZSTD_CStream* cstream; - ZSTDMT_jobDescription jobs[1]; /* variable size (must lies at the end) */ }; -ZSTDMT_CCtx *ZSTDMT_createCCtx(unsigned nbThreads) +ZSTDMT_CCtx* ZSTDMT_createCCtx(unsigned nbThreads) { ZSTDMT_CCtx* cctx; U32 const minNbJobs = nbThreads + 2; @@ -306,7 +306,7 @@ ZSTDMT_CCtx *ZSTDMT_createCCtx(unsigned nbThreads) DEBUGLOG(5, "nbThreads : %u ; minNbJobs : %u ; nbJobsLog2 : %u ; nbJobs : %u \n", nbThreads, minNbJobs, nbJobsLog2, nbJobs); if ((nbThreads < 1) | (nbThreads > ZSTDMT_NBTHREADS_MAX)) return NULL; - cctx = (ZSTDMT_CCtx*) calloc(1, sizeof(ZSTDMT_CCtx) + nbJobs*sizeof(ZSTDMT_jobDescription)); + cctx = (ZSTDMT_CCtx*) calloc(1, sizeof(ZSTDMT_CCtx)); if (!cctx) return NULL; cctx->nbThreads = nbThreads; cctx->jobIDMask = nbJobs - 1; @@ -314,9 +314,10 @@ ZSTDMT_CCtx *ZSTDMT_createCCtx(unsigned nbThreads) cctx->sectionSize = 0; cctx->overlapRLog = 3; cctx->factory = POOL_create(nbThreads, 1); + cctx->jobs = malloc(nbJobs * sizeof(*cctx->jobs)); cctx->buffPool = ZSTDMT_createBufferPool(nbThreads); cctx->cctxPool = ZSTDMT_createCCtxPool(nbThreads); - if (!cctx->factory | !cctx->buffPool | !cctx->cctxPool) { /* one object was not created */ + if (!cctx->factory | !cctx->jobs | !cctx->buffPool | !cctx->cctxPool) { ZSTDMT_freeCCtx(cctx); return NULL; } @@ -356,6 +357,7 @@ size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx) POOL_free(mtctx->factory); if (!mtctx->allJobsCompleted) ZSTDMT_releaseAllJobResources(mtctx); /* stop workers first */ ZSTDMT_freeBufferPool(mtctx->buffPool); /* release job resources into pools first */ + free(mtctx->jobs); ZSTDMT_freeCCtxPool(mtctx->cctxPool); ZSTD_freeCDict(mtctx->cdict); ZSTD_freeCStream(mtctx->cstream); @@ -491,7 +493,8 @@ size_t ZSTDMT_compressCCtx(ZSTDMT_CCtx* mtctx, /* ======= Streaming API ======= */ /* ====================================== */ -static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* zcs) { +static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* zcs) +{ while (zcs->doneJobID < zcs->nextJobID) { unsigned const jobID = zcs->doneJobID & zcs->jobIDMask; PTHREAD_MUTEX_LOCK(&zcs->jobCompleted_mutex); From b6dec4c3ae18786d6183274b3bcb5fe7387ea2bc Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 27 May 2017 17:09:06 -0700 Subject: [PATCH 3/3] fixed minor cast warning --- lib/compress/zstdmt_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index d41825e8..0c6bc724 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -314,7 +314,7 @@ ZSTDMT_CCtx* ZSTDMT_createCCtx(unsigned nbThreads) cctx->sectionSize = 0; cctx->overlapRLog = 3; cctx->factory = POOL_create(nbThreads, 1); - cctx->jobs = malloc(nbJobs * sizeof(*cctx->jobs)); + cctx->jobs = (ZSTDMT_jobDescription*) malloc(nbJobs * sizeof(*cctx->jobs)); cctx->buffPool = ZSTDMT_createBufferPool(nbThreads); cctx->cctxPool = ZSTDMT_createCCtxPool(nbThreads); if (!cctx->factory | !cctx->jobs | !cctx->buffPool | !cctx->cctxPool) {