Skip to content

Commit 436d5a4

Browse files
dloeblCopilot
andauthored
add malloc failure test for cgif_rgb API (dloebl#95)
* add malloc failure test for cgif_rgb API * rgb: add missing alloc error checks * make the rgb test cover more allocs * address review feedback from other PRs * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * improve test * simplify ealloc_rgb test --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent f729ed6 commit 436d5a4

3 files changed

Lines changed: 268 additions & 27 deletions

File tree

src/cgif_rgb.c

Lines changed: 107 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -126,40 +126,60 @@ static int32_t col_hash_collision_count(const uint8_t* rgb, const uint8_t* hashT
126126
return h;
127127
}
128128

129+
/* free the color hash table*/
130+
static void free_col_hash_table(colHashTable* colhash){
131+
free(colhash->frequ);
132+
free(colhash->hashTable);
133+
free(colhash->indexUsed);
134+
free(colhash->pPalette);
135+
free(colhash->colIdx);
136+
free(colhash);
137+
}
138+
129139
/* initialize hash table storing colors and their frequency */
130140
static colHashTable* init_col_hash_table(uint32_t tableSize){
131141
colHashTable* colhash = malloc(sizeof(colHashTable));
142+
if(colhash == NULL) {
143+
return NULL;
144+
}
145+
memset(colhash, 0, sizeof(colHashTable));
132146
colhash->tableSize = getNextPrimePower2(tableSize); // increase table size to next prime number
133147
colhash->frequ = malloc(sizeof(uint32_t) * colhash->tableSize);
134148
colhash->hashTable = malloc(3 * colhash->tableSize);
135149
colhash->indexUsed = malloc(colhash->tableSize);
136150
colhash->pPalette = malloc(3 * colhash->tableSize);
137151
colhash->colIdx = malloc(sizeof(uint32_t)*colhash->tableSize);
152+
if(!colhash->frequ || !colhash->hashTable || !colhash->indexUsed || !colhash->pPalette || !colhash->colIdx) {
153+
free_col_hash_table(colhash);
154+
return NULL;
155+
}
138156
colhash->cnt = 0; // no colors initially
139157
memset(colhash->pPalette, 0, 3 * colhash->tableSize); // unused part of color table is uninitialized otheriwse
140158
memset(colhash->indexUsed, 0, colhash->tableSize); // initially no entry in hash-table is used
141159
return colhash;
142160
}
143161

144-
/* free the color hash table*/
145-
static void free_col_hash_table(colHashTable* colhash){
146-
free(colhash->frequ);
147-
free(colhash->hashTable);
148-
free(colhash->indexUsed);
149-
free(colhash->pPalette);
150-
free(colhash->colIdx);
151-
free(colhash);
152-
}
153-
154162
/* increase the size of the color hash table */
155-
static void resize_col_hash_table(colHashTable* colhash){
163+
static int resize_col_hash_table(colHashTable* colhash){
156164
uint32_t tableSizeNew;
157165
tableSizeNew = getNextPrimePower2(colhash->tableSize); // increase table size to the next prime number above the next power of two
158-
colhash->pPalette = realloc(colhash->pPalette, 3 * tableSizeNew);
159-
colhash->colIdx = realloc(colhash->colIdx, sizeof(uint32_t) * tableSizeNew);
166+
uint8_t* pPaletteNew = realloc(colhash->pPalette, 3 * tableSizeNew);
167+
uint32_t* colIdxNew = realloc(colhash->colIdx, sizeof(uint32_t) * tableSizeNew);
160168
uint8_t* hashTable_new = malloc(3 * tableSizeNew);
161169
uint8_t* indexUsed_new = malloc(tableSizeNew);
162-
uint32_t* frequ_new = malloc(sizeof(uint32_t) * tableSizeNew);
170+
uint32_t* frequ_new = malloc(sizeof(uint32_t) * tableSizeNew);
171+
if(!pPaletteNew || !colIdxNew || !hashTable_new || !indexUsed_new || !frequ_new) {
172+
// On allocation failure, apply any successful reallocs to colhash and free
173+
// any newly allocated auxiliary buffers before returning an error.
174+
if(pPaletteNew) colhash->pPalette = pPaletteNew;
175+
if(colIdxNew) colhash->colIdx = colIdxNew;
176+
free(hashTable_new);
177+
free(indexUsed_new);
178+
free(frequ_new);
179+
return -1;
180+
}
181+
colhash->pPalette = pPaletteNew;
182+
colhash->colIdx = colIdxNew;
163183
memset(indexUsed_new, 0, tableSizeNew);
164184
colhash->cnt = 0;
165185
for(uint32_t j = 0; j < colhash->tableSize; ++j) { // TBD (no improvement when tested): easier to loop over pPalette and also leave pPalette in place?, if indexUsed is also unnecessary then
@@ -180,13 +200,17 @@ static void resize_col_hash_table(colHashTable* colhash){
180200
colhash->hashTable = hashTable_new; // pass pointer to new hash table
181201
colhash->indexUsed = indexUsed_new; // pass pointer to new hash table
182202
colhash->frequ = frequ_new; // pass pointer to new hash table
203+
return 0;
183204
}
184205

185206
/* take frequ indexed by hash(rgb) and return corresponding dense array */
186207
static uint32_t* hash_to_dense(colHashTable* colhash, cgif_chan_fmt fmtChan) {
187208
uint32_t* frequDense = malloc(sizeof(uint32_t) * colhash->cnt);
188209
uint32_t h;
189210
(void)fmtChan;
211+
if(frequDense == NULL) {
212+
return NULL;
213+
}
190214
for(uint32_t i = 0; i < colhash->cnt; ++i) {
191215
h = col_hash(colhash->pPalette + 3 * i, colhash->hashTable, colhash->indexUsed, colhash->tableSize, 3);
192216
frequDense[i] = colhash->frequ[h];
@@ -243,6 +267,9 @@ static treeNode* new_tree_node(uint8_t* pPalette, uint32_t* frequ, uint16_t* num
243267
float var[3];
244268

245269
treeNode* node = malloc(sizeof(treeNode));
270+
if(node == NULL) {
271+
return NULL;
272+
}
246273
node->idxMin = idxMin; // minimum color in pPalette belonging to the node
247274
node->idxMax = idxMax; // maximum color in pPalette belonging to the node
248275
get_variance(pPalette, frequ, idxMin, idxMax, var, node->mean);
@@ -254,7 +281,7 @@ static treeNode* new_tree_node(uint8_t* pPalette, uint32_t* frequ, uint16_t* num
254281
}
255282

256283
/* create the decision tree. (Similar to qsort with limited depth: pPalette, frequ get sorted) */
257-
static void crawl_decision_tree(treeNode* root, uint16_t* numLeaveNodes, uint8_t* pPalette, uint32_t* frequ, uint16_t colMax) {
284+
static int crawl_decision_tree(treeNode* root, uint16_t* numLeaveNodes, uint8_t* pPalette, uint32_t* frequ, uint16_t colMax) {
258285
uint32_t i, k, saveNum;
259286
uint16_t nodeIdx = 0;
260287
uint8_t saveBlk[3];
@@ -283,10 +310,14 @@ static void crawl_decision_tree(treeNode* root, uint16_t* numLeaveNodes, uint8_t
283310
(*numLeaveNodes)--; // decrease counter since parent is removed as a leave node
284311
parent->child0 = new_tree_node(pPalette, frequ, numLeaveNodes, parent->idxMin, i - 1, parent->colIdx); // i-1 is last index of 1st block, one child takes color index from parent
285312
parent->child1 = new_tree_node(pPalette, frequ, numLeaveNodes, i, parent->idxMax, *numLeaveNodes);
313+
if(parent->child0 == NULL || parent->child1 == NULL) {
314+
return -1;
315+
}
286316
nodeList[2*(*numLeaveNodes) - 3] = parent->child0; // add new child nodes to the list (total number of nodes is always 2*(*numLeaveNodes)-1)
287317
nodeList[2*(*numLeaveNodes) - 2] = parent->child1; // add new child nodes to the list (total number of nodes is always 2*(*numLeaveNodes)-1)
288318
}
289319
}
320+
return 0;
290321
}
291322

292323
/* fill 256 color table using the decision tree */
@@ -314,25 +345,34 @@ static uint8_t get_leave_node_index(const treeNode* root, const float* rgb) {
314345
}
315346
}
316347

317-
/* color quantization with mean cut method (TBD? switch to median cut)
318-
(works with dense palette, no hash table) */
319-
static treeNode* create_decision_tree(uint8_t* pPalette, uint32_t* pFrequDense, uint8_t* pPalette256, uint32_t cnt, uint16_t colMax, uint8_t depthMax){
320-
uint16_t numLeaveNodes = 0;
321-
treeNode* root = new_tree_node(pPalette, pFrequDense, &numLeaveNodes, 0, cnt - 1, 0);
322-
crawl_decision_tree(root, &numLeaveNodes, pPalette, pFrequDense, colMax);
323-
get_palette_from_decision_tree(root, pPalette256); // fill the reduced color table
324-
return root;
325-
}
326-
327348
/* free memory allocated for the tree */
328349
static void free_decision_tree(treeNode* root){
350+
if(root == NULL) {
351+
return;
352+
}
329353
if(root->isLeave == 0) { // if the node has children
330354
free_decision_tree(root->child0);
331355
free_decision_tree(root->child1);
332356
}
333357
free(root); // free root once free is called for children
334358
}
335359

360+
/* color quantization with mean cut method (TBD? switch to median cut)
361+
(works with dense palette, no hash table) */
362+
static treeNode* create_decision_tree(uint8_t* pPalette, uint32_t* pFrequDense, uint8_t* pPalette256, uint32_t cnt, uint16_t colMax, uint8_t depthMax){
363+
uint16_t numLeaveNodes = 0;
364+
treeNode* root = new_tree_node(pPalette, pFrequDense, &numLeaveNodes, 0, cnt - 1, 0);
365+
if(root == NULL) {
366+
return NULL;
367+
}
368+
if(crawl_decision_tree(root, &numLeaveNodes, pPalette, pFrequDense, colMax) != 0) {
369+
free_decision_tree(root);
370+
return NULL;
371+
}
372+
get_palette_from_decision_tree(root, pPalette256); // fill the reduced color table
373+
return root;
374+
}
375+
336376
/* get image with max 256 color indices using Floyd-Steinberg dithering */
337377
static void get_quantized_dithered_image(uint8_t* pImageData, float* pImageDataRGBfloat, uint8_t* pPalette256, treeNode* root, uint32_t numPixel, uint32_t width, uint8_t dithering, uint8_t transIndex, cgif_chan_fmt fmtChan, uint8_t* pBef, cgif_chan_fmt befFmtChan, int hasAlpha) {
338378
// pImageData: image with (max 256) color indices (length: numPixel)
@@ -446,6 +486,9 @@ static colHashTable* get_color_histogram(const uint8_t* pImageDataRGB, uint32_t
446486
uint32_t cntCollision; // count the number of collision
447487
uint32_t tableSize = 262147; // initial size of the hash table
448488
colHashTable* colhash = init_col_hash_table(tableSize); // initialize the hash table storing all the colors
489+
if(colhash == NULL) {
490+
return NULL;
491+
}
449492
*pHasAlpha = 0; // assume no alpha channel until it is found
450493
const uint8_t sizePixel = fmtChan; // number of bytes for one pixel (e.g. 3 for RGB, 4 for RGBa)
451494
for(uint32_t i = 0; i < numPixel; ++i) {
@@ -466,14 +509,17 @@ static colHashTable* get_color_histogram(const uint8_t* pImageDataRGB, uint32_t
466509
}
467510
// resize the hash table (if more than half-full)
468511
if((colhash->cnt > (colhash->tableSize >> 1) || cntCollision > MAX_COLLISIONS) && colhash->tableSize < MAX_TABLE_SIZE) {
469-
resize_col_hash_table(colhash);
512+
if(resize_col_hash_table(colhash) != 0) {
513+
free_col_hash_table(colhash);
514+
return NULL;
515+
}
470516
}
471517
}
472518
return colhash;
473519
}
474520

475521
/* quantize the image using the color histogram */
476-
static uint32_t quantize_and_dither(colHashTable* colhash, const uint8_t* pImageDataRGB, uint32_t numPixel, uint32_t width, cgif_chan_fmt fmtChan, uint8_t* pImageData, uint8_t* pPalette256, uint8_t depthMax, uint8_t dithering, int hasAlpha, uint8_t* pBef, cgif_chan_fmt befFmtChan) {
522+
static int quantize_and_dither(colHashTable* colhash, const uint8_t* pImageDataRGB, uint32_t numPixel, uint32_t width, cgif_chan_fmt fmtChan, uint8_t* pImageData, uint8_t* pPalette256, uint8_t depthMax, uint8_t dithering, int hasAlpha, uint8_t* pBef, cgif_chan_fmt befFmtChan) {
477523
// pImageDataRGB: image in RGB format
478524
// numPixel: number of pixels of input image
479525
// width: width of the image (needed for dithering)
@@ -488,9 +534,19 @@ static uint32_t quantize_and_dither(colHashTable* colhash, const uint8_t* pImage
488534
const uint16_t colMax = (1uL << depthMax) - 1; // maximum number of colors (-1 to leave space for transparency), disadvantage (TBD): quantization for static image with 256 colors and no alpha channel unnecessary
489535
if(colhash->cnt > colMax) { // color-quantization is needed
490536
uint32_t* pFrequDense = hash_to_dense(colhash, fmtChan);
537+
if(pFrequDense == NULL) {
538+
return -1;
539+
}
491540
treeNode* root = create_decision_tree(colhash->pPalette, pFrequDense, pPalette256, colhash->cnt, colMax, depthMax); // create decision tree (dynamic, splits along rgb-dimension with highest variance)
492541
free(pFrequDense);
542+
if(root == NULL) {
543+
return -1;
544+
}
493545
float* pImageDataRGBfloat = malloc(fmtChan * numPixel * sizeof(float)); // TBD fmtChan + only when hasAlpha
546+
if(pImageDataRGBfloat == NULL) {
547+
free_decision_tree(root);
548+
return -1;
549+
}
494550
for(uint32_t i = 0; i < fmtChan * numPixel; ++i){
495551
pImageDataRGBfloat[i] = pImageDataRGB[i];
496552
}
@@ -521,6 +577,9 @@ CGIFrgb* cgif_rgb_newgif(const CGIFrgb_Config* pConfig) {
521577
CGIFrgb* pGIFrgb;
522578

523579
pGIFrgb = malloc(sizeof(CGIFrgb));
580+
if(pGIFrgb == NULL) {
581+
return NULL;
582+
}
524583
memset(pGIFrgb, 0, sizeof(CGIFrgb));
525584
idxConfig.pWriteFn = pConfig->pWriteFn;
526585
idxConfig.pContext = pConfig->pContext;
@@ -558,19 +617,40 @@ cgif_result cgif_rgb_addframe(CGIFrgb* pGIF, const CGIFrgb_FrameConfig* pConfig)
558617
return CGIF_ERROR;
559618
}
560619
pNewBef = malloc(pConfig->fmtChan * MULU16(imageWidth, imageHeight));
620+
if(pNewBef == NULL) {
621+
pGIF->curResult = CGIF_EALLOC;
622+
return pGIF->curResult;
623+
}
561624
memcpy(pNewBef, pConfig->pImageData, pConfig->fmtChan * MULU16(imageWidth, imageHeight));
562625
fConfig.pLocalPalette = aPalette;
563626
fConfig.pImageData = malloc(pGIF->config.width * (uint32_t)pGIF->config.height);
627+
if(fConfig.pImageData == NULL) {
628+
free(pNewBef);
629+
pGIF->curResult = CGIF_EALLOC;
630+
return pGIF->curResult;
631+
}
564632
fConfig.delay = pConfig->delay;
565633
fConfig.attrFlags = CGIF_FRAME_ATTR_USE_LOCAL_TABLE;
566634
if(pConfig->attrFlags & CGIF_RGB_FRAME_ATTR_INTERLACED) {
567635
fConfig.attrFlags |= CGIF_FRAME_ATTR_INTERLACED;
568636
}
569637

570638
colHashTable* colhash = get_color_histogram(pConfig->pImageData, numPixel, pConfig->fmtChan, &hasAlpha);
639+
if(colhash == NULL) {
640+
free(pNewBef);
641+
free(fConfig.pImageData);
642+
pGIF->curResult = CGIF_EALLOC;
643+
return pGIF->curResult;
644+
}
571645
const uint8_t bDither = !(pConfig->attrFlags & CGIF_RGB_FRAME_ATTR_NO_DITHERING);
572646
const int sizeLCT = quantize_and_dither(colhash, pConfig->pImageData, numPixel, pGIF->config.width, pConfig->fmtChan, fConfig.pImageData, aPalette, 8, bDither, hasAlpha, pGIF->pBefImageData, pGIF->befFmtChan);
573647
free_col_hash_table(colhash);
648+
if(sizeLCT < 0) {
649+
free(pNewBef);
650+
free(fConfig.pImageData);
651+
pGIF->curResult = CGIF_EALLOC;
652+
return pGIF->curResult;
653+
}
574654

575655
fConfig.numLocalPaletteEntries = sizeLCT;
576656
if(hasAlpha) {

0 commit comments

Comments
 (0)