4

I would like to know why the first two elements are always non-zero. I don't know how more I can describe the question, but this is not allowing me to post the question, so I'm writing this. Not sure if this will work.

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

#define SIZE 3

void printMatrix(int **m)
{
    for (int i = 0; i < SIZE; i++) {
        for (int j = 0; j < SIZE; j++)
            printf("%d ", m[i][j]);
        printf("\n");
    }
}

int main(int argc, char const *argv[])
{
    int **matrix;

    matrix = (int **) calloc(sizeof(int), SIZE);

    for (int i = 0; i < SIZE; ++i)
        matrix[i] = (int *) calloc(sizeof(int), SIZE);

    printf("%s\n", "Matrix initialized.");

    printMatrix(matrix);

    return 0;
}

Output:

Matrix initialized.
1371548192 32653 0 
0 0 0 
0 0 0
Nikhil CSB
  • 155
  • 9
  • 1
    Can you update your first call to calloc as `matrix = (int **) calloc(sizeof(int*), SIZE);` and let me know if there is any change? – Phalgun Sep 29 '19 at 15:26
  • Yes, it worked after I changed it to `sizeof(int *)`. I realized it will be 8 on my system as opposed to `sizeof(int)` which is 4. – Nikhil CSB Sep 29 '19 at 15:37

3 Answers3

4

You're not allocating enough memory:

matrix = (int **) calloc(sizeof(int), SIZE);

Here you're attempting to create an array of 3 int *, but you're only allocating space for 3 int. If a pointer is larger than an int on your system, which it most likely is, you write past the end of the array when you create the arrays for each row. Writing past the end of allocated memory invokes undefined behavior.

Since you're creating an array of int *, use that for the size of each element:

matrix = calloc(sizeof(int *), SIZE);

Also, don't cast the return value of malloc/realloc/calloc, as that can mask a bug if you forget to #include <stdlib.h>

dbush
  • 205,898
  • 23
  • 218
  • 273
3

The code uses calloc(sizeof(int), SIZE), but the actual datatype in the structure is int *, leading to insufficient memory allocation on some systems (mine gives int size as 4 and int * size as 8).

Here's a rewrite suggestion (we'll swap size parameters in the calloc call per its header):

int main(int argc, char const *argv[]) {
    int **matrix;

    if (!(matrix = calloc(SIZE, sizeof(*matrix)))) { 
        fprintf(stderr, "calloc failed");
        return 1;
    }

    for (int i = 0; i < SIZE; ++i) {
         if (!(matrix[i] = calloc(SIZE, sizeof(*(matrix[i]))))) {
            fprintf(stderr, "calloc failed");
            return 1;
        }
    }

    printf("%s\n", "Matrix initialized.");
    printMatrix(matrix);
    return 0;
}

Here, we use *matrix and *matrix[i] instead of hard coding the types int * and int respectively. This can help us avoid bugs and hunting for locations to change code if we need to make type adjustments at some point.

We also check that calloc succeeds by testing that the pointer is non-NULL. Failing to do so can introduce difficult-to-find bugs due to undefined behavior.

Note Do I cast the result of malloc?.

ggorlen
  • 44,755
  • 7
  • 76
  • 106
  • `sizeof(*matrix[i])` scares me a little. Is that `sizeof( (*matrix)[i])` or `sizeof( * (matrix[i]))`? Certainly the rules dictate one or the other, yet `sizeof *(matrix[i])` leave no doubt to the `p = malloc(sizeof *(p) * n)` idiom. – chux - Reinstate Monica Sep 29 '19 at 17:02
2

Reference manual describes calloc as:

void* calloc (size_t num, size_t size);

So, calloc takes firstly num of elements and then size of a particular element

Try:

matrix = (int **) calloc(SIZE, sizeof(int *));
  • 2
    Note that the issue is not the ordering of the parameters (it ends up not mattering, although it is good that you suggest to use the proper one), but the `sizeof(int)`. – Acorn Sep 29 '19 at 15:20
  • @Acorn I agree with you in some general way. The clue is to allocate memory for next pointer, instead of the type of values. Size of pointer depends on the system settings and then this is a reason why we have got an undefined behavior. – BrockMarekins Sep 29 '19 at 15:31