Skip to content

Commit

Permalink
Miscellaneous fixes (#893)
Browse files Browse the repository at this point in the history 
* Typos in unittests error messages
* Incorrect flag usage and error messages in texturetests. Fixes #888.
* Memory not freed after deflate and inflate errors. Fixes #892.
  • Loading branch information
@MarkCallow
MarkCallow authored Apr 11, 2024
1 parent eac4b98 commit e31b3e5
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 33 deletions.
32 changes: 25 additions & 7 deletions lib/texture2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2494,6 +2494,7 @@ ktxTexture2_inflateZstdInt(ktxTexture2* This, ktx_uint8_t* pDeflatedData,
ktxLevelIndexEntry* cindex = This->_private->_levelIndex;
ktxLevelIndexEntry* nindex;
ktx_uint32_t uncompressedLevelAlignment;
ktx_error_code_e result;

ZSTD_DCtx* dctx;

Expand All @@ -2515,6 +2516,10 @@ ktxTexture2_inflateZstdInt(ktxTexture2* This, ktx_uint8_t* pDeflatedData,

ktx_size_t inflatedByteLength = 0;
dctx = ZSTD_createDCtx();
if (dctx == NULL) {
result = KTX_OUT_OF_MEMORY;
goto cleanup;
}
for (int32_t level = This->numLevels - 1; level >= 0; level--) {
size_t levelByteLength =
ZSTD_decompressDCtx(dctx, pInflatedData + levelOffset,
Expand All @@ -2525,13 +2530,17 @@ ktxTexture2_inflateZstdInt(ktxTexture2* This, ktx_uint8_t* pDeflatedData,
ZSTD_ErrorCode error = ZSTD_getErrorCode(levelByteLength);
switch(error) {
case ZSTD_error_dstSize_tooSmall:
return KTX_DECOMPRESS_LENGTH_ERROR; // inflatedDataCapacity too small.
result = KTX_DECOMPRESS_LENGTH_ERROR; // inflatedDataCapacity too small.
goto cleanup;
case ZSTD_error_checksum_wrong:
return KTX_DECOMPRESS_CHECKSUM_ERROR;
result = KTX_DECOMPRESS_CHECKSUM_ERROR;
goto cleanup;
case ZSTD_error_memory_allocation:
return KTX_OUT_OF_MEMORY;
default:
return KTX_FILE_DATA_ERROR;
result = KTX_OUT_OF_MEMORY;
goto cleanup;
default:
result = KTX_FILE_DATA_ERROR;
goto cleanup;
}
}

Expand Down Expand Up @@ -2562,6 +2571,11 @@ ktxTexture2_inflateZstdInt(ktxTexture2* This, ktx_uint8_t* pDeflatedData,
bdb[KHR_DF_WORD_BYTESPLANE0] = prtctd->_formatSize.blockSizeInBits / 8;

return KTX_SUCCESS;

cleanup:
ZSTD_freeDCtx(dctx);
free(nindex);
return result;
}

/**
Expand Down Expand Up @@ -2616,11 +2630,15 @@ ktxTexture2_inflateZLIBInt(ktxTexture2* This, ktx_uint8_t* pDeflatedData,
&levelByteLength,
&pDeflatedData[cindex[level].byteOffset],
cindex[level].byteLength);
if (result != KTX_SUCCESS)
if (result != KTX_SUCCESS) {
free(nindex);
return result;
}

if (This->_private->_levelIndex[level].uncompressedByteLength != levelByteLength)
if (This->_private->_levelIndex[level].uncompressedByteLength != levelByteLength) {
free(nindex);
return KTX_DECOMPRESS_LENGTH_ERROR;
}

nindex[level].byteOffset = levelOffset;
nindex[level].uncompressedByteLength = nindex[level].byteLength =
Expand Down
33 changes: 25 additions & 8 deletions lib/writer2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,11 @@ ktxTexture2_DeflateZstd(ktxTexture2* This, ktx_uint32_t compressionLevel)
ktxLevelIndexEntry* cindex = This->_private->_levelIndex;
ktxLevelIndexEntry* nindex;
ktx_uint8_t* pCmpDst;
ktx_error_code_e result;

ZSTD_CCtx* cctx = ZSTD_createCCtx();
if (cctx == NULL)
return KTX_OUT_OF_MEMORY;

if (This->supercompressionScheme != KTX_SS_NONE)
return KTX_INVALID_OPERATION;
Expand All @@ -797,8 +800,10 @@ ktxTexture2_DeflateZstd(ktxTexture2* This, ktx_uint32_t compressionLevel)
}

workBuf = malloc(dstRemainingByteLength + levelIndexByteLength);
if (workBuf == NULL)
return KTX_OUT_OF_MEMORY;
if (workBuf == NULL) {
result = KTX_OUT_OF_MEMORY;
goto cleanup;
}
nindex = (ktxLevelIndexEntry*)workBuf;
pCmpDst = &workBuf[levelIndexByteLength];

Expand All @@ -814,28 +819,33 @@ ktxTexture2_DeflateZstd(ktxTexture2* This, ktx_uint32_t compressionLevel)
ZSTD_ErrorCode error = ZSTD_getErrorCode(levelByteLengthCmp);
switch(error) {
case ZSTD_error_parameter_outOfBound:
return KTX_INVALID_VALUE;
result = KTX_INVALID_VALUE;
goto cleanup;
case ZSTD_error_dstSize_tooSmall:
#ifdef DEBUG
assert(false && "Deflate dstSize too small.");
#else
return KTX_OUT_OF_MEMORY;
result = KTX_OUT_OF_MEMORY;
goto cleanup;
#endif
case ZSTD_error_workSpace_tooSmall:
#ifdef DEBUG
assert(false && "Deflate workspace too small.");
#else
return KTX_OUT_OF_MEMORY;
result = KTX_OUT_OF_MEMORY;
goto cleanup;
#endif
case ZSTD_error_memory_allocation:
return KTX_OUT_OF_MEMORY;
result = KTX_OUT_OF_MEMORY;
goto cleanup;
default:
// The remaining errors look like they should only
// occur during decompression but just in case.
#ifdef DEBUG
assert(true);
#else
return KTX_INVALID_OPERATION;
result = KTX_INVALID_OPERATION;
goto cleanup;
#endif
}
}
Expand Down Expand Up @@ -868,6 +878,11 @@ ktxTexture2_DeflateZstd(ktxTexture2* This, ktx_uint32_t compressionLevel)
bdb[KHR_DF_WORD_BYTESPLANE0] = 0; /* bytesPlane3..0 = 0 */

return KTX_SUCCESS;

cleanup:
ZSTD_freeCCtx(cctx);
free(workBuf);
return result;
}

/**
Expand Down Expand Up @@ -919,8 +934,10 @@ ktxTexture2_DeflateZLIB(ktxTexture2* This, ktx_uint32_t compressionLevel)
&This->pData[cindex[level].byteOffset],
cindex[level].byteLength,
compressionLevel);
if (result != KTX_SUCCESS)
if (result != KTX_SUCCESS) {
free(workBuf);
return result;
}

nindex[level].byteOffset = levelOffset;
nindex[level].uncompressedByteLength = cindex[level].byteLength;
Expand Down
32 changes: 16 additions & 16 deletions tests/texturetests/texturetests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2579,12 +2579,12 @@ TEST_F(ktxTexture2_MetadataTest, EmptyValue) {

if (ktxMemFile != NULL) {
result = ktxTexture2_CreateFromMemory(ktxMemFile, ktxMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

result = ktxHashList_AddKVPair(&texture->kvDataHead,
"MSCtestKey", 0, nullptr);
Expand All @@ -2599,12 +2599,12 @@ TEST_F(ktxTexture2_MetadataTest, EmptyValue) {
ktxTexture_Destroy(ktxTexture(texture));

result = ktxTexture2_CreateFromMemory(newMemFile, newMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

ktx_uint32_t valueLen;
ktx_uint8_t* value;
Expand All @@ -2628,12 +2628,12 @@ TEST_F(ktxTexture2_MetadataTest, NoMetadata) {

if (ktxMemFile != NULL) {
result = ktxTexture2_CreateFromMemory(ktxMemFile, ktxMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

ktxHashList_Destruct(&texture->kvDataHead);
ktxTexture(texture)->kvDataHead = nullptr;
Expand All @@ -2651,12 +2651,12 @@ TEST_F(ktxTexture2_MetadataTest, NoMetadata) {
ktxTexture_Destroy(ktxTexture(texture));

result = ktxTexture2_CreateFromMemory(newMemFile, newMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

ktx_uint32_t valueLen;
ktx_uint8_t* value;
Expand All @@ -2678,12 +2678,12 @@ TEST_F(ktxTexture2_MetadataTest, NoLibVersionDupOnMultipleWrites) {

if (ktxMemFile != NULL) {
result = ktxTexture2_CreateFromMemory(ktxMemFile, ktxMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

const ktx_uint32_t iterations = 2;
ktx_size_t newMemFileLens[iterations];
Expand All @@ -2707,12 +2707,12 @@ TEST_F(ktxTexture2_MetadataTest, NoLibVersionDupOnMultipleWrites) {
ktx_uint8_t* value;
result = ktxTexture2_CreateFromMemory(newMemFiles[i],
newMemFileLens[i],
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

result = ktxHashList_FindValue(&texture->kvDataHead,
"KTXwriter",
Expand Down Expand Up @@ -2746,12 +2746,12 @@ TEST_F(ktxTexture2_MetadataTest, LibVersionUpdatedCorrectly) {

if (ktxMemFile != NULL) {
result = ktxTexture2_CreateFromMemory(ktxMemFile, ktxMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

ktx_uint32_t curWriterLen;
ktx_uint8_t* curWriterVal;
Expand Down Expand Up @@ -2792,12 +2792,12 @@ TEST_F(ktxTexture2_MetadataTest, LibVersionUpdatedCorrectly) {
ktx_uint8_t* newWriterVal;
result = ktxTexture2_CreateFromMemory(newMemFile,
newMemFileLen,
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image data not loaded";

result = ktxHashList_FindValue(&texture->kvDataHead,
"KTXwriter",
Expand Down
4 changes: 2 additions & 2 deletions tests/unittests/unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,9 @@ class SwizzleTestBase : public ::testing::Test {
KTX_TEXTURE_CREATE_ALLOC_STORAGE,
&texture);
ASSERT_TRUE(result == KTX_SUCCESS);
ASSERT_TRUE(texture != NULL) << "ktxTexture_CreateFromMemory failed: "
ASSERT_TRUE(texture != NULL) << "ktxTexture_Create failed: "
<< ktxErrorString(result);
ASSERT_TRUE(texture->pData != NULL) << "Image stoage not allocated";
ASSERT_TRUE(texture->pData != NULL) << "Image storage not allocated";

result = helper.copyImagesToTexture(ktxTexture(texture));
ASSERT_TRUE(result == KTX_SUCCESS);
Expand Down

0 comments on commit e31b3e5

Please sign in to comment.