Reputation: 411
I'm creating an image compressor for a project. I generate codes for the values in the image in such a way that for every grey value (from 0-254) there is a char* code in an array called codeArray (Huffman Encoding).
A requirement is to have a function which returns unsigned char*. I go through every pixel and convert the grey value of that pixel to a code using the codeArray.
I need to make the unsigned char array grow dynamically as more grey values are converted and concatenated to the end of the array.
unsigned char* encodedString = malloc(sizeof(char));
int width = image->width; //width and height of image structure
int height = image->height;
int row, col;
for(row = 0; row<height; row++)
for(col = 0; col<width; col++)
{
int value = image->pixel[row][col]; //gets the grey value
encodedString = realloc(encodedString, (strlen(encodedString)+strlen(codeArray[value])));
strcat(encodedString, codeArray[value]);
}
I tried running this with a print statement after the strcat and found it printed until there were 24 characters then started printing garbage and then Seg faulted.
Help appreciated!
Upvotes: 2
Views: 1048
Reputation: 613192
You call strlen(encodedString)
on an uninitialized buffer. This is undefined behaviour. You need to zero-terminate the initial contents of encodedString
.
unsigned char* encodedString = malloc(1);
//check for malloc errors
encodedString[0] = '\0';
It looks like you get away with that error, but then immediately commit another one. Your realloc
makes space for strlen(encodedString)+strlen(codeArray[value])
but you have forgotten to allocate room for the zero terminator. Presumably that's what causes strcat
to bomb. Fix that problem by adding one to the size parameter to realloc
.
As @Lou points out the performance of your realloc strategy may be poor. You may be better to allocate the buffer once at the beginning of the function since presumably you can put a relatively tight upper bound on its size.
And you also should not ever write ptr = realloc(ptr, ...)
since you won't be able to recover from a failure of realloc
and will always leak. But that's really a nuance in comparison to the other faults.
Upvotes: 4