Nick Schudlo
Nick Schudlo

Reputation: 411

Allocating space and concatenating to a unsigned char array in c

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

Answers (1)

David Heffernan
David Heffernan

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

Related Questions