Gibberish data being stored in memory while concatenating the strings using `strcat` function

Question

With the code below at strcat functions, I could see that only 4-5 bytes of the first string data is correct and rest of the data is some junk that is getting stored in memory. What might be the issue here and what can be done to overcome and concatenate all the strings together?

char* fileHeader = createBitmapFileHeader(height, stride);  //14 bytes
char* infoHeader = createBitmapInfoHeader(height, width);   //40 bytes
char* pixels;
pixels = (char*)malloc(246*sizeof(char));
pixels[0]='\0';
strcat(pixels,fileHeader);
strcat(pixels,infoHeader);
strcat(pixels,image); //image size is 192 bytes

unsigned char* createBitmapFileHeader (int height, int stride)
{
    int fileSize = FILE_HEADER_SIZE + INFO_HEADER_SIZE + (stride * height);

    static unsigned char fileHeader[] = {
        0,0,     /// signature
        0,0,0,0, /// image file size in bytes
        0,0,0,0, /// reserved
        0,0,0,0, /// start of pixel array
    };
    fileHeader[ 0] = (unsigned char)('B');
    fileHeader[ 1] = (unsigned char)('M');
    fileHeader[ 2] = (unsigned char)(fileSize      );
    fileHeader[ 3] = (unsigned char)(fileSize >>  8);
    fileHeader[ 4] = (unsigned char)(fileSize >> 16);
    fileHeader[ 5] = (unsigned char)(fileSize >> 24);
    fileHeader[10] = (unsigned char)(FILE_HEADER_SIZE + INFO_HEADER_SIZE);
    return fileHeader;
}

unsigned char* createBitmapInfoHeader (int height, int width)
{
    static unsigned char infoHeader[] = {
        0,0,0,0, /// header size
        0,0,0,0, /// image width
        0,0,0,0, /// image height
        0,0,     /// number of color planes
        0,0,     /// bits per pixel
        0,0,0,0, /// compression
        0,0,0,0, /// image size
        0,0,0,0, /// horizontal resolution
        0,0,0,0, /// vertical resolution
        0,0,0,0, /// colors in color table
        0,0,0,0, /// important color count
    };

    infoHeader[ 0] = (unsigned char)(INFO_HEADER_SIZE);
    infoHeader[ 4] = (unsigned char)(width      );
    infoHeader[ 5] = (unsigned char)(width >>  8);
    infoHeader[ 6] = (unsigned char)(width >> 16);
    infoHeader[ 7] = (unsigned char)(width >> 24);
    infoHeader[ 8] = (unsigned char)(height      );
    infoHeader[ 9] = (unsigned char)(height >>  8);
    infoHeader[10] = (unsigned char)(height >> 16);
    infoHeader[11] = (unsigned char)(height >> 24);
    infoHeader[12] = (unsigned char)(1);
    infoHeader[14] = (unsigned char)(BYTES_PER_PIXEL*8);

    return infoHeader;
}

//Don't care about freeing dynamic memory now, I'm doing it somewhere else.

Screenshot of CreateBitmapFileHeader and CreateBitmapInfoHeader

Screenshot of Gibberish data that being accommodated at pixel memory

The data pointed to by `pixels` are never initialized (maybe you want `calloc` instead of `malloc`) - so, the first `strcat` give undefined behaviour, and all other operations are equally undefined. — Adrian Mole, Nov 05 '21 at 10:37
Also, using `strcat` on data that aren't strings is poor practice. How do you know that there aren't any zero bytes in the BMP headers? — Adrian Mole, Nov 05 '21 at 10:39
Assuming your `createBitmapFileHeader` actually makes a `BITMAPFILEHEADER` structure (and similarly for `createBitmapInfoHeader`), then there ***will*** be embedded zeros in the structure. Maybe clarify what you're actually trying to do here? — Adrian Mole, Nov 05 '21 at 10:44
Thanks for the response @Adrian Mole How do you know that there aren't any zero bytes in the BMP headers?. Yes, there are some zero Bytes in BitmapHeader. `CreateBitmapFileHeader` and `createBitmapInfoHeader` makes structure only that include zeros. — Yuvi, Nov 05 '21 at 11:00
See the answer in the linked missing null terminator FAQ. `strcat` expects a null-terminated string as first parameter but you aren't providing one. — Lundin, Nov 05 '21 at 11:01
Other than that, you seem to think that anything pointed at by a character type is a string. This is wrong, raw binary data isn't strings and you shouldn't treat it as char but rather an unsigned byte, `uint8_t` is recommended. — Lundin, Nov 05 '21 at 11:03

score -1 · Answer 1 · answered Nov 05 '21 at 10:37

-1

pixels is not initiated and will be filled with random data that is already left in the memmory. If you want it to be an empty string you can simply do pixels[0] = '\0'.

answered Nov 05 '21 at 10:37

Girocired

57
1
5

This is true, but the _actual_ problem here is trying to use `strcat` for binary data. – Jabberwocky Nov 05 '21 at 11:14
The question states that: "What might be the issue here and what can be done to overcome and concatenate all the >>strings<< together?", so the question does state that they are strings, not binary data. – Girocired Nov 05 '21 at 11:25
Please do feel free to check the edited Question. – Yuvi Nov 05 '21 at 11:44
You should try using `void *memcpy(void *dest, const void * src, size_t n)` for binary data, because strcpy will stop at first 0 or '\0' if we look at it as char thinking it is the end of the string. I would though recommend you return how many bytes both create functions return so that you can put that as "size_t n" param, otherwise for test you can use fixed values. Example: `memcpy(pixels, fileHeader, 14 * sizeof(char)); memcpy(pixels + 14, infoHeader, 40 * sizeof(char));` I assumed that the number of bytes written in comments is correct. – Girocired Nov 05 '21 at 11:51
@Girocired if you read the rest of the code, you'll see that it's binary data – Jabberwocky Nov 05 '21 at 12:50
Thanks for the suggestion @Girocired Pixels size is already defined as 246 (`14 - FileHeader` , `40 - InfoHeader` , `192 - image` ). In `memcpy(pixels, fileHeader, 14 * sizeof(char)); memcpy(pixels + 14, infoHeader, 40 * sizeof(char));` Why are we adding `+14` at second line ? Because while putting all data `(info, file, image together in Pixels)`, could see that some of the information from `fileHeader` is missing, why is this happening? – Yuvi Nov 10 '21 at 06:06
`memcpy(pixels, fileHeader, 14 * sizeof(char));` puts 14 bytes into pixels, so when we add infoHeader we need to offset the array by 14 bytes that we already inserted, otherwise we will write over what fileHeader inserted. – Girocired Nov 10 '21 at 09:43

Gibberish data being stored in memory while concatenating the strings using `strcat` function

1 Answers1