Integer value changes during insertion to array

Question

I am having some weird problems with my code and I have no idea what I'm doing wrong. I'm relatively new to C-language, so I'd really appreciate if someone could point out if I'm doing something stupid.

I'm trying to read data from a file into two dimensional array of type state. Type state is defined as follows:

typedef uint8_t state [4][4];

I have defined method read_block that gets the file pointer as parameter and returns pointer two dimensional type state array which contains first 16 bytes from that file.

state* read_block(FILE *fp) {
    state *arr = (state*) malloc(sizeof (state));
    uint8_t *temp = (uint8_t*) malloc(sizeof (uint8_t)*16);

    fread(temp, 16, 1, fp);

    /*
    for (int y = 0; y < 16; y++) {
        printf("%u ", temp[y]);
    }
    printf("\n");
    */

    int n = 0;
    for (int i = 0; i < 4; i++) {
        for (int j = 0; j < 4; j++) {
            /*
            printf("%u ", temp[n]);
            */
            *arr[i][j] = temp[n];
            n++;
        }
    }

    return arr;
}

Code compiles and runs ok, but the type state array that method returns always contains one wrong value. Specifically, the value in position [3][0] is always wrong.

The test file I'm trying to read contains numbers:

0 1 2 3 0 1 2 3 0 1 2 3 0 1 2 3

I decided to check if the values are read right and if I remove comments around printf lines I get as output:

0 1 2 3 0 1 2 3 0 1 2 3 0 1 2 3 
0 1 2 3 0 1 2 3 0 1 2 3 3 1 2 3

In which the first line is exactly correct (that is printed right after fread()), but for some reason the second line is incorrect (that is printed when inserting values into arr).

I can't figure out what is causing this. I've also tried reading values one byte at time with fread, but the same value keeps being wrong. I'm using NetBeans, so I'm not sure if this is somehow being caused by it.

Answer 1

You're running into operator precedence.

*arr[i][j]

is the same as

*(arr[i][j])   // or arr[i][j][0]

This is the same only if i == 0, i.e. in the first line.

What you want is

(*arr)[i][j]

Answer 2

It is not really a good idea to hide an array behind a typedef. This is the root of the problem, since arr is actually an array pointer of type uint8_t (*)[4][4]. And when de-referencing an array pointer you have to first take the contents of the pointer to get the array, then after access a specific index. That is, you will have to do (*arr)[i][j] rather than *arr[i][j].

To avoid that fishy syntax in turn, you could use a trick and declare an array pointer to the outermost dimension: uint8_t(*)[4]. Then you can suddenly type arr[i][j] instead. arr[i] then means do pointer arithmetic on a uint8_t(*)[4] type, and then in the obtained result you can access a specific array index, [j].

In addition to the operator precedence issue, you probably want to store data directly into the array. There's no reason why you can't do fread(arr, 16, 1, fp);, since you are using that array in the same order as it is allocated.

I haven't tested it, but you should be able to replace the whole function with this:

void read_block(FILE* fp, uint8_t (*arr)[4][4]) 
{
  const size_t size = sizeof(uint8_t[4][4]);
  *arr = malloc(size);
  fread(*arr, size, 1, fp);
}