Trouble Storing a Pointer as a Two Dimensional Array

Question

First off, this is a snippet of code from my larger project, so please excuse the lack of error checks and other minor details. I excluded these details for readability.
This snippet is supposed to read a .raw file and store the data into a uint16_t * then convert it into a 2D array which needs to be written back as an identical file using fwrite().
I know for a fact that the data being written into image_ptr is correct. I believe my problem is occuring when I am attempting to save the variable image_ptr into the 2D array named image in the main function or it is occurring within the fwrite() statement in my save_image function.

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

#define MAX_ROW 2560
#define MAX_COL 2160

uint16_t image[MAX_ROW][MAX_COL];

uint16_t* get_image_data(uint32_t image_index)
{
    int result;
    char filename[32];
    sprintf(filename, "image%d.raw", image_index);
    
    FILE *image_raw = fopen(filename, "rb");
    fseek(image_raw, 0, SEEK_END);
    long filesize = ftell(image_raw);
    rewind(image_raw);
    long num_samples = filesize / sizeof(uint16_t);

    /*READ IMAGE DATA*/
    uint16_t *image_data_ptr = (uint16_t*) malloc(filesize);
    size_t num_read = fread(image_data_ptr, sizeof(uint16_t), num_samples, image_raw);
    fclose(image_raw);
    
    return image_data_ptr;
}

int save_image()
{
    int row, col, pixel_index = 0;
    int data_size = MAX_ROW * MAX_COL;
    uint16_t * image_data = malloc((sizeof *image_data)*data_size);
    image_data = image[0];
    FILE *ptr_image_file;
    ptr_image_file = fopen("image21.raw", "wb");
    
    for(col = 0; col < MAX_COL; col++) 
    {
        if(**(image + col) != 0)
        {
            if (fwrite(*(image + col), sizeof(uint16_t), MAX_ROW, ptr_image_file) != MAX_ROW) 
            {
                fprintf(stderr, "Can't write to output file\n");
                fclose(ptr_image_file);
                return -1;
            }
        }
    }
    
    fclose(ptr_image_file);
    return 0;
}

int main()
{
    uint16_t * image_ptr = malloc((sizeof *image_ptr)*MAX_ROW*MAX_COL);
    //Get Image Data
    image_ptr = get_image_data(1);
    
    int row, col, pixel_index = 0;
    for(col = 0; col < MAX_COL; col++) 
    {
        for(row = 0; row < MAX_ROW; row++) 
        {
            *(*(image + col) + row) = *(image_ptr + pixel_index);
            pixel_index++; 
        }
    }
    
    save_image();
    
    return 0;
}

You should only have one `malloc` in this entire sequence (namely, in `get_image_data`). Allocate space so you can read the file data into memory, then pass that pointer to whatever other functions need that data, including the one that writes the data back to file. — yano, Jul 21 '21 at 21:00
, you have a memory leak with `uint16_t * image_ptr = malloc(...); image_ptr = get_image_data(1);` The pointer returned by `malloc` is overwritten by the pointer returned by `get_image_data`, so that original memory block is reserved but cannot be referenced. As with your [question yesterday](https://stackoverflow.com/questions/68458089/storing-raw-file-data-as-a-pointer-using-c), why are you using `uint16_t*`? Looks like to me you just want raw bytes, you could be running into an endianess issue writing 2 byte pairs back to file. — yano, Jul 21 '21 at 21:04
@yano If I do not use malloc before the function call then I get a segmentation fault. I am using uint16_t because the 2D array `image` is of that type. The `image` is supposed to be receiving 16-bit data over comms. — RMarms, Jul 21 '21 at 21:36
@SergeyA I'm not sure what you mean by two dimensional arrays do not exist in C. There are plenty of resources online talking about and I can easily store and manipulate data within my code. — RMarms, Jul 21 '21 at 21:40
@RMarms this is a popular misconception. C doesn't have multidimensional arrays. Instead, it has a single-dimensional arrays with any type you like. For example, you can have an array of arrays - which for simplicity is often presented as a multi-dim array, but is not as far as C type system is concerned, and although in many cases behaves as-if it was a multi-dim array, in other cases does not. — SergeyA, Jul 21 '21 at 21:43
then you have problems elsewhere, UB that's manifesting itself in different ways. `get_image_data` `malloc`s memory and returns a pointer to it. Your first two lines in `main` should be replaced with `uint16_t* image_ptr = get_image_data(1);` (although I still think you should use `uint8_t*`). Your first `malloc` in `main` as is does nothing but leak memory. I admittedly don't know anything about .raw image format, but the image data is nothing but bytes, grouping them in pairs is only going to add confusion. What if your image has an odd number of bytes? — yano, Jul 21 '21 at 21:46
@yano My image does have an odd number of bytes (2927445 bytes). When I simply `fwrite` it back it results in 2927444 bytes (missing 1 byte). However, this doesn't solve my overall issue and I found this result using a method I cannot use for this test. — RMarms, Jul 21 '21 at 22:04
@SergeyA That's interesting, thanks for the info, so when you say "You can't store a pointer as two dimensional array" are you saying I cannot parse the data from the pointer and store it into my 2D array (array of arrays)? Or were you simply just saying that `uint16_t * != uint16_t[][]`? — RMarms, Jul 21 '21 at 22:25
@SergeyA *C doesn't have multidimensional arrays.* Yes it does. Per [**6.5.2.1 Array subscripting**, paragraph 3 of the C 11 standard](https://port70.net/~nsz/c/c11/n1570.html#6.5.2.1p3) (bolding mine): "Successive subscript operators designate an element of a **multidimensional array** object. If E is an **n-dimensional array (n >= 2)** with dimensions i x j x . . . x k, then E (used as other than an lvalue) is converted to a pointer to an **(n - 1)-dimensional array** ..." — Andrew Henle, Jul 22 '21 at 02:35

score 1 · Answer 1 · answered Jul 21 '21 at 21:35

Your code is trying to use a global image buffer and buffers from malloc in several places, causing memory leaks.

Your transform using pixel_index is unclear as to intent.

You're using pointers to the wrong buffers in most cases, working on images that have no data.

Your pointer indexing is incorrect.

Here is a refactored version of the code.

I've used cpp conditionals to denote old vs. new code:

#if 0
// old code
#else
// new code
#endif

The code has annotations for the bugs. I've compiled it but not tested it:

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

#define MAX_ROW 2560
#define MAX_COL 2160

#if 0
uint16_t image[MAX_ROW][MAX_COL];
#endif

uint16_t *
get_image_data(uint32_t image_index)
{
#if 0
    int result;
#endif
    char filename[32];

    sprintf(filename, "image%d.raw", image_index);

    FILE *image_raw = fopen(filename, "rb");

    fseek(image_raw, 0, SEEK_END);
    long filesize = ftell(image_raw);

    rewind(image_raw);

// NOTE/BUG: what if filesize is odd?
    long num_samples = filesize / sizeof(uint16_t);

    /* READ IMAGE DATA */
// NOTE/BUG: don't cast the return value of malloc
#if 0
    uint16_t *image_data_ptr = (uint16_t *) malloc(filesize);
#else
    uint16_t *image_data_ptr = malloc(filesize);
#endif
    size_t num_read = fread(image_data_ptr, sizeof(uint16_t), num_samples,
        image_raw);
    if (num_read != num_samples) {
        perror("fread");
        exit(1);
    }

    fclose(image_raw);

    return image_data_ptr;
}

int
save_image(uint16_t *image_ptr)
{
#if 0
    int row, col, pixel_index = 0;
    int data_size = MAX_ROW * MAX_COL;
#else
    int row;
#endif

// NOTE/BUG: this blows away the pointer you just got from malloc and created
// a memory leak
// NOTE/BUG: this allocated array has no valid data
#if 0
    uint16_t *image_data = malloc((sizeof *image_data) * data_size);
    image_data = image[0];
#endif
    FILE *ptr_image_file;

    ptr_image_file = fopen("image21.raw", "wb");

#if 0
    for (col = 0; col < MAX_COL; col++) {
// NOTE/BUG: should always write the data
        if (**(image + col) != 0) {
// NOTE/BUG: wrong way to index into image
            if (fwrite(*(image + col), sizeof(uint16_t), MAX_ROW, ptr_image_file) != MAX_ROW) {
                fprintf(stderr, "Can't write to output file\n");
                fclose(ptr_image_file);
                return -1;
            }
        }
    }
#else
    // output image row-by-row
    for (row = 0; row < MAX_ROW; row++) {
        // output single row
        if (fwrite(&image_ptr[row * MAX_COL], sizeof(uint16_t), MAX_COL,
            ptr_image_file) != MAX_COL) {
            fprintf(stderr, "Can't write to output file\n");
            fclose(ptr_image_file);
            return -1;
        }
    }
#endif

    fclose(ptr_image_file);

    return 0;
}

int
main(void)
{
// NOTE/BUG: this is blown away with the call to get_image_data
#if 0
    uint16_t *image_ptr = malloc((sizeof *image_ptr) * MAX_ROW * MAX_COL);
#else
    uint16_t *image_ptr;
#endif

    // Get Image Data
    image_ptr = get_image_data(1);

    int row,
     col,
     pixel_index = 0;

// NOTE/BUG: this uses the global variable image which has no data
// NOTE/BUG: for loops make access very cache intensive (i.e. slow)
// NOTE/BUG: what transform are you trying to do?
#if 0
    for (col = 0; col < MAX_COL; col++) {
        for (row = 0; row < MAX_ROW; row++) {
// NOTE/BUG: wrong way to index
            *(*(image + col) + row) = *(image_ptr + pixel_index);
            pixel_index++;
        }
    }
#else
    for (row = 0; row < MAX_ROW; row++) {
        for (col = 0; col < MAX_COL; col++) {
            image_ptr[(row * MAX_COL) + col] += pixel_index;
            pixel_index++;
        }
    }
#endif

#if 0
    save_image();
#else
    save_image(image_ptr);
    free(image_ptr);
#endif

    return 0;
}

Thanks for your response Craig. When I get rid of the `malloc` it results in a segmentation fault, which is the reason I added it initially. Also `pixel_index` is used to iterate through `image_ptr` and adds each value at the address to the 2D array. — RMarms, Jul 21 '21 at 22:09
Use [work from] the code I wrote--it works (I just tested it). I surmised the intent of the `pixel_index` transform. In your original code, indexing into `image` _and_ `image_ptr` were _both_ broken. Understand the changes made [and why] as this will help you a lot. For example, especially, compare the indexing of the arrays between your code and mine. — Craig Estey, Jul 21 '21 at 22:46

Trouble Storing a Pointer as a Two Dimensional Array

1 Answers1