C dynamically allocated struct object causes runtime error ("Segmentation fault (core dumped)")

Question

struct matrix {
    int m, n;   // m rows, n columns
    int** data;
    char name[64];
};

struct matrix* alloc_matrix(const char* name, int m, int n) {
    struct matrix *mat = malloc(sizeof(struct matrix));
    strcpy(mat->name, name);
    mat->m = m;
    mat->n = n;
    mat->data = (int**)malloc(sizeof(m) * (m * n));
    return mat;
}

int main() {
    struct matrix* mat1 = alloc_matrix("mat1", 4, 4);
    mat1->data[0][0] = 2;    <------------------------------ This line causes the error
    return EXIT_SUCCESS;
}

So I want to implement matrices. I defined a struct matrix that holds the name, rows count, columns count and data of a matix. With a function alloc_matrix I want to allocade memory for a struct object. But something goes wrong in this function, because if I want to retrieve or set data on a allocated object, I get a runtime memory error.

I hope someone has more experience with dynamic data allocation and sees the problem.

Answer 1

mat->data = (int**)malloc(sizeof(m) * (m * n)); this line does not do what you think. You need to alloc space for the pointers first, then space for the referenced objects for all of allocated pointers.

important!!! You need always to check the result of the malloc!

it has to be:

    if(mat->data = malloc(sizeof(*mat->data) * (m)))
    {
        for(size_t index = 0; index < n; index++)
        {
            if(!(mat->data[index] = malloc(sizeof(**mat->data) * n)))
            {
                //do something if malloc failed
            }
        }
    }

https://godbolt.org/z/skBJzb

Personally I would not do it this way in this case. I prefere personally to limit number of mallocs

struct matrix {
    size_t r, c;   // r rows, c columns
    char name[64];
    int data[];
};

struct matrix* alloc_matrix(const char* name, size_t rows, size_t cols) {
    struct matrix *mat = malloc(sizeof(*mat) + rows * cols * sizeof(mat -> data[0]));
    if(mat)
    {
        strcpy(mat->name, name);
        mat->r = rows;
        mat->c = cols;
    }
    return mat;
}

int setdata(struct matrix *mat, size_t row, size_t col, int val)
{
    int result = -1;
    
    if(mat)
    {
        if(row < mat -> r && col < mat -> c)
        {
            mat -> data[row * mat -> c + col] = val;
            result = 0;
        }
    }
    return result;
}


int main() {
    struct matrix* mat1 = alloc_matrix("mat1", 4, 4);
    
    if(mat1)
    {
        printf("set result %d\n", setdata(mat1, 2, 2, 0));
        printf("set result %d\n", setdata(mat1, 5, 2, 0));
    }
}

Answer 2

data is being used as a two-dimension array. That is, it's an array where each element of the array is itself an array of ints.

mat->data = (int**)malloc(sizeof(m) * (m * n));

This line allocates the first array. It now has enough space to hold m*n pointers, but the pointers don't point to anything. This isn't what you wanted.

What you want is for data to hold m pointers, each of which hold n elements.

mat->data = (int**)malloc(sizeof(*mat->data) * m);

if(!mat->data)
{
    //handle error case
}

for(int ii = 0; ii < m; ++ii)
{
    mat->data[ii] = (int*)malloc(sizeof(**mat->data) * n);#

    if(!mat->data[ii])
    {
         //handle error case
    }
}