From 89c97d5ea682ba7e2481d610a8b7a2b057a80391 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 30 May 2019 17:29:51 -0700 Subject: [PATCH 1/3] fullbench: added test scenario LZ4F_decompress_followHint This emulates a streaming scenario, where the caller follows rigorously the srcSize hints provided as return value of LZ4F_decompress(). This is useful to show the issue in #714, where data is uselessly copied in a tmp buffer first. --- tests/fullbench.c | 64 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/tests/fullbench.c b/tests/fullbench.c index d2af662..2488a54 100644 --- a/tests/fullbench.c +++ b/tests/fullbench.c @@ -343,11 +343,45 @@ static int local_LZ4F_decompress(const char* in, char* out, int inSize, int outS assert(inSize >= 0); assert(outSize >= 0); result = LZ4F_decompress(g_dCtx, out, &dstSize, in, &srcSize, NULL); - if (result!=0) { DISPLAY("Error decompressing frame : unfinished frame\n"); exit(8); } - if (srcSize != (size_t)inSize) { DISPLAY("Error decompressing frame : read size incorrect\n"); exit(9); } + if (result!=0) { DISPLAY("Error decompressing frame : unfinished frame \n"); exit(8); } + if (srcSize != (size_t)inSize) { DISPLAY("Error decompressing frame : read size incorrect \n"); exit(9); } return (int)dstSize; } +static int local_LZ4F_decompress_followHint(const char* src, char* dst, int srcSize, int dstSize) +{ + size_t totalInSize = (size_t)srcSize; + size_t maxOutSize = (size_t)dstSize; + + size_t inPos = 0; + size_t inSize = 0; + size_t outPos = 0; + size_t outRemaining = maxOutSize - outPos; + + do { + size_t const sizeHint = LZ4F_decompress(g_dCtx, dst+outPos, &outRemaining, src+inPos, &inSize, NULL); + assert(!LZ4F_isError(sizeHint)); + + inPos += inSize; + inSize = sizeHint; + + outPos += outRemaining; + outRemaining = maxOutSize - outPos; + + if (!sizeHint) break; + + } while(1); + + /* frame completed */ + if (inPos != totalInSize) { + DISPLAY("Error decompressing frame : must read (%u) full frame (%u) \n", + (unsigned)inPos, (unsigned)totalInSize); + exit(10); + } + return (int)outPos; + +} + #define NB_COMPRESSION_ALGORITHMS 100 #define NB_DECOMPRESSION_ALGORITHMS 100 @@ -446,7 +480,14 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) for (i=0; i g_chunkSize) { chunkP[i].origSize = g_chunkSize; remaining -= g_chunkSize; } else { chunkP[i].origSize = (int)remaining; remaining = 0; } + assert(g_chunkSize > 0); + if (remaining > (size_t)g_chunkSize) { + chunkP[i].origSize = g_chunkSize; + remaining -= (size_t)g_chunkSize; + } else { + chunkP[i].origSize = (int)remaining; + remaining = 0; + } chunkP[i].compressedBuffer = out; out += maxCompressedChunkSize; chunkP[i].compressedSize = 0; } @@ -565,17 +606,14 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) #ifndef LZ4_DLL_IMPORT case 8: decompressionFunction = local_LZ4_decompress_safe_forceExtDict; dName = "LZ4_decompress_safe_forceExtDict"; break; #endif - case 9: decompressionFunction = local_LZ4F_decompress; dName = "LZ4F_decompress"; - { size_t const errorCode = LZ4F_compressFrame(compressed_buff, compressedBuffSize, orig_buff, benchedSize, NULL); - if (LZ4F_isError(errorCode)) { - DISPLAY("Error while preparing compressed frame\n"); - free(orig_buff); - free(compressed_buff); - free(chunkP); - return 1; - } + case 10: + case 11: + if (dAlgNb == 10) { decompressionFunction = local_LZ4F_decompress; dName = "LZ4F_decompress"; } /* can be skipped */ + if (dAlgNb == 11) { decompressionFunction = local_LZ4F_decompress_followHint; dName = "LZ4F_decompress_followHint"; } /* can be skipped */ + { size_t const fcsize = LZ4F_compressFrame(compressed_buff, (size_t)compressedBuffSize, orig_buff, benchedSize, NULL); + assert(!LZ4F_isError(fcsize)); chunkP[0].origSize = (int)benchedSize; - chunkP[0].compressedSize = (int)errorCode; + chunkP[0].compressedSize = (int)fcsize; nbChunks = 1; break; } From 99f1721ff5e3ab74fbe1fc8c68337ba87c1a8be9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 31 May 2019 12:01:33 -0700 Subject: [PATCH 2/3] replaced while(1) by for (;;) just to please Visual Studio C4127 . --- tests/fullbench.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/fullbench.c b/tests/fullbench.c index 2488a54..dbdb02c 100644 --- a/tests/fullbench.c +++ b/tests/fullbench.c @@ -358,7 +358,7 @@ static int local_LZ4F_decompress_followHint(const char* src, char* dst, int srcS size_t outPos = 0; size_t outRemaining = maxOutSize - outPos; - do { + for (;;) { size_t const sizeHint = LZ4F_decompress(g_dCtx, dst+outPos, &outRemaining, src+inPos, &inSize, NULL); assert(!LZ4F_isError(sizeHint)); @@ -369,8 +369,7 @@ static int local_LZ4F_decompress_followHint(const char* src, char* dst, int srcS outRemaining = maxOutSize - outPos; if (!sizeHint) break; - - } while(1); + } /* frame completed */ if (inPos != totalInSize) { From 33a04fb8bd19fb4450ea37dad6aa0fd0d7f4007c Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 31 May 2019 13:25:12 -0700 Subject: [PATCH 3/3] fullbench: ensure decompressionFunction and dName are initialized Visual Studio seems to miss that they are necessarily initialized in the switch() { case: } --- tests/fullbench.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/fullbench.c b/tests/fullbench.c index dbdb02c..4609f13 100644 --- a/tests/fullbench.c +++ b/tests/fullbench.c @@ -571,9 +571,15 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) nbChunks = (int) (((int)benchedSize + (g_chunkSize-1))/ g_chunkSize); for (i=0; i g_chunkSize) { chunkP[i].origSize = g_chunkSize; remaining -= g_chunkSize; } else { chunkP[i].origSize = (int)remaining; remaining = 0; } + if ((int)remaining > g_chunkSize) { + chunkP[i].origSize = g_chunkSize; + remaining -= (size_t)g_chunkSize; + } else { + chunkP[i].origSize = (int)remaining; + remaining = 0; + } chunkP[i].compressedBuffer = out; out += maxCompressedChunkSize; chunkP[i].compressedSize = 0; } @@ -586,8 +592,8 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) /* Decompression Algorithms */ for (dAlgNb=0; (dAlgNb <= NB_DECOMPRESSION_ALGORITHMS) && g_decompressionTest; dAlgNb++) { - const char* dName; - int (*decompressionFunction)(const char*, char*, int, int); + const char* dName = NULL; + int (*decompressionFunction)(const char*, char*, int, int) = NULL; double bestTime = 100000000.; int checkResult = 1; @@ -609,6 +615,7 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) case 11: if (dAlgNb == 10) { decompressionFunction = local_LZ4F_decompress; dName = "LZ4F_decompress"; } /* can be skipped */ if (dAlgNb == 11) { decompressionFunction = local_LZ4F_decompress_followHint; dName = "LZ4F_decompress_followHint"; } /* can be skipped */ + /* prepare compressed data using frame format */ { size_t const fcsize = LZ4F_compressFrame(compressed_buff, (size_t)compressedBuffSize, orig_buff, benchedSize, NULL); assert(!LZ4F_isError(fcsize)); chunkP[0].origSize = (int)benchedSize; @@ -620,6 +627,9 @@ int fullSpeedBench(const char** fileNamesTable, int nbFiles) continue; /* skip if unknown ID */ } + assert(decompressionFunction != NULL); + assert(dName != NULL); + { size_t i; for (i=0; i