0

I have a piece of code that stores bytes from an image into a multidimensional array stored in a struct. While assigning the bytes to the array both the expected data (0xFF since I am testing with a pure white image) and the data that is actually pointed to by the array at pixel_array[i][j][k] match. Here is a snippet of the code that stores data in the array:

unsigned char pixel[bmp_image->pixel_width];
bmp_image->pixel_array = malloc(bmp_image->width * sizeof(unsigned char));

for (int i = 0; i < bmp_image->width; i++) {
    bmp_image->pixel_array[i] = malloc(bmp_image->height * sizeof(unsigned char));

    for (int j = 0; j < bmp_image->height; j++) {
        bmp_image->pixel_array[i][j] = malloc(bmp_image->pixel_width * sizeof(unsigned char));
        fread(pixel, sizeof(pixel), 1, bmp_data);

        // Pixel width refers to the size of a pixel in bytes
        for (int k = 0; k < bmp_image->pixel_width; k++) {
            bmp_image->pixel_array[i][j][k] = pixel[k];
            // These two print statements always result in the same data (i.e. both print 0xFF for the white test image)
            printf("Expected byte: %x\n", pixel[k]);
            printf("Saved byte: %x\n\n", bmp_image->pixel_array[i][j][k]);
        }
    }

    fseek(bmp_data, padding, SEEK_CUR);
}

And here is the struct storing image data:

struct Image {
    FILE *image_file;
    int width;
    int height;
    int pixel_width;
    int error_code;
    unsigned char*** pixel_array;
};

My issue is that as soon as I try to access chars stored in the pixel array using the following code I get random unexpected output (i.e. not hex FF)

for (int i = 0; i < bmp_image->width; i++) {
    for (int j = 0; j < bmp_image->height; j++) {
        for (int k = 0; k < bmp_image->pixel_width; k++) {
            printf("%x", bmp_image->pixel_array[i][j][k]);
        }
    }
}

An example of the output with a 10x10 white image can be seen below (expected to be only ff):

6050a3c051a32053a38054a3e055a34057a3ffffffffffffffffffffffffc04ca3404da3fffffffffffffffffffffffff
fffffffffffffffffffffff204ea3a04ea3ffffffffffffffffffffffffffffffffffffffffffffffff804fa3050a3fff
fffffffffffffffffffffffffffffffffffffffffffffe050a36051a3ffffffffffffffffffffffffffffffffffffffff
ffffffff4052a3c052a3ffffffffffffffffffffffffffffffffffffffffffffffffa053a32054a3fffffffffffffffff
fffffffffffffffffffffffffffffff055a38055a3ffffffffffffffffffffffffffffffffffffffffffffffff6056a3e
056a3ffffffffffffffffffffffffffffffffffffffffffffffffc057a34058a3ffffffffffffffffffffffffffffffff
ffffffffffffffff

The random data appears to always be in the same position, but the values vary each time. I am assuming I am running into some issue with incorrectly assign pointers but as of now I am stumped as to what could be causing it.

Ethan Peck
  • 37
  • 6
  • 2
    Code is incomplete. For example, how is `pixel_array` allocated? Questions seeking debugging help must provide a [complete minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example). Please update the question with code that anyone can take exactly as shown to reproduce the problem. – kaylum Aug 23 '21 at 21:35
  • `malloc(bmp_image->height * sizeof(unsigned char)` should use `sizeof(unsigned char*)` – Barmar Aug 23 '21 at 21:37
  • 1
    Safer is `sizeof *bmp_image->pixel_array[i]`. If you just copy the variable you're assigning to and precede it with `*`, the size will always be correct no matter what its type is. – Barmar Aug 23 '21 at 21:38
  • And as @kaylum suggested, there could be a similar problem when allocating `bmp_image->pixel_array`. – Barmar Aug 23 '21 at 21:40
  • Apologies @kaylum I've added the line where I allocate pixel_array now – Ethan Peck Aug 23 '21 at 22:00
  • 1
    `unsigned char*** pixel_array;` <<-- three star programmer alert! – wildplasser Aug 23 '21 at 22:02
  • @wildplasser I'm still fairly new to c and would love to hear ways to improve. I know having very indirect pointers is usually a bad thing but I'm not quite sure what ways would be best to handle multidimensional arrays. – Ethan Peck Aug 23 '21 at 22:12
  • 1
    Well: the *golden rule* is: if you need mere than two stars, you probably need *some kind* of structure. Or a different data-model. [there are exceptions to any rule] – wildplasser Aug 23 '21 at 23:00

1 Answers1

1

It looks that you allocate not enough memory.

bmp_image->pixel_array[i] = malloc(bmp_image->height * sizeof(unsigned char));

It allocates memory for bmp_image->height while it should allocate resources for bmp_image->height pointers. A good pattern that helps avoiding such errors is:

x = malloc(sizeof *x);

For this case it would be:

bmp_image->pixel_array[i] = malloc(bmp_image->height * sizeof(*bmp_image->pixel_array[i]));

I assume that bmp_image->pixel_array was correctly allocated before use.

tstanisl
  • 13,520
  • 2
  • 25
  • 40
  • This worked perfectly thank you! So to make sure I'm understanding this right, arrays in c are technically storing pointers to the value at that index in memory? So since the char type is typically smaller in size than a pointer that was why not enough memory was allocated and I was accessing random unallocated junk values? – Ethan Peck Aug 23 '21 at 22:07
  • 1
    @Ethan Peck, it is a bit more complex. You can find quite detailed explanation at https://stackoverflow.com/a/918121/4989451 as well as in the other answers in that thread. – tstanisl Aug 23 '21 at 23:03