Allocating memory for a structure within a function which returns pointer to struct

Question

I'm trying to create a struct which represents a two dimensional mathematical matrix.

The function 'initMatrix' is intended to initialise a matrix of n rows by n columns, with the elements stored in a dynamically allocated array of doubles and set to zero.

However when I try to assign zero to the array by dereferencing the 'data' pointer within the struct (according to this question), this causes the program to fail.

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

typedef struct
{
  int rows;
  int cols;
  double *data;
} Matrix;

Matrix *initMatrix(int rows, int cols)
{
  Matrix *ptr;
  ptr = (Matrix *)malloc(sizeof(int) * 2 + sizeof(double) * rows * cols);
  if(ptr == NULL) printf("Couldn't allocate memory");
  for(int i = 0; i < rows; i++) {
    for(int j = 0; j < cols; j++) {
      *(ptr->data) = 0; //error originating here
      ptr->data++;
    }
  }
  return ptr;
}

int main(void) 
{
  Matrix *mptr = initMatrix(2, 2);

  return 0;
}

What am I doing wrong?

Answer 1

As data is a pointer to some memory region, you will have to malloc this region separately, i.e.

ptr = (Matrix *)malloc(sizeof(Matrix));
ptr->data = (double *)malloc(sizeof(double)*rows*cols);

Pay attention, that you will also have to call free on mptr->data as well as on mptr in the end to return all heap-allocated memory.

That being said, you might want to simplify your program by returning a Matrix instead of Matrix * from initMatrix. Then, you will only have to care for allocating/freeing the data member:

Matrix initMatrix(int rows, int cols)
{
  Matrix result;
  result.data = (double *)malloc(sizeof(double) * rows * cols);

  if(result.data == NULL) printf("Couldn't allocate memory");

  result.rows = rows;
  result.cols = cols;

  // initialization of result.data here, consider using memset() ...
  return result;
}

To initialize the memory to zero, you might want to use memset instead of a discrete loop.

Answer 2

ptr = (Matrix *)malloc(sizeof(int) * 2 + sizeof(double) * rows * cols);

you should do this for ptr->data, not for ptr.

Dereferencing data fails because you never allocated memory for it, so it's pointing nowhere (undefined behavior, to be precise).

So first allocate a Matrix, then allocate what's needed for data

ptr = malloc(sizeof(Matrix));
ptr -> data = malloc(sizeof(int) * 2 + sizeof(double) * rows * cols);

Answer 3

You need to allocate memory for the data separately and for the matrix separately. Also you should free the memory allocated inside the initMatrix function in your main().

In addition to what all the other very good answers have mentioned about using memset() or calloc() instead of doing this yourself in 2 for loops, and so on, I want to mention a couple of things which the other posters have not answered, and also to capture the gist what I sense the OP was asking:

1. You are confusing a matrix which needs a double **data with a 1-d vector that contains the same number of elements and can be stored in double *data.

   That is why you are using the two for loops, I guess. You don't want 4 elements in a 1 row. You want 2 rows and 2 cols! You can look at the code below to see how that can be achieved.

2. Remember to free both the mptr and the data.

3. In addition in your code, you are not initializing ptr->cols and ptr->rows.

Code:

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

typedef struct
{
    int rows;
    int cols;
    double **data;
} Matrix;

Matrix *initMatrix(int rows, int cols)
{
    Matrix *ptr;
    ptr = (Matrix *)malloc(sizeof(Matrix));
    ptr->cols = cols;
    ptr->rows = rows;
    ptr->data = (double**)malloc(sizeof(double*) * rows);
    for(int i = 0; i < rows; i++) {
        ptr->data[i] = (double*)malloc(sizeof(double) * cols);
        for(int j = 0; j < cols; j++) {
            ptr->data[i][j] = i*10+j;
            // printf("%d,%d,%lf", i, j, ptr->data[i][j]); // just for debugging
        }
    }
    return ptr;
}

int main(void)
{
    Matrix *mptr = initMatrix(2, 2);
    printf("rows:%d, cols:%d\n", mptr->rows, mptr->cols);

    for (int i=0; i<2; i++) {
        for (int j=0; j<2; j++) {
            printf("element[%d][%d]:%lf\n",i,j,mptr->data[i][j]);
        }
    }

    free(mptr->data);
    free(mptr);

    return 0;
}

which produces the output:

rows:2, cols:2
element[0][0]:0.000000
element[0][1]:1.000000
element[1][0]:10.000000
element[1][1]:11.000000

which i just displayed to debug. You can change the line ptr->data[i][j] = i*10+j; and get whatever output you want, including all zeros.

Answer 4

If you want to allocate the whole structure with one malloc you could use

typedef struct
{
  int rows;
  int cols;
  double data[1];
} Matrix;

In contrast to the other answers this allows to use a single free.

If your compiler supports arrays of size 0 you can also use double data[0];.

From C99 onward you can use an array without dimension in the structure: double data[];, see https://stackoverflow.com/a/2061038/10622916

Instead of calculating the size for malloc by adding the size of the structure fields' types you should better use sizeof or offsetof with the structure type Matrix because there may be some padding. (Probably not in your case but this depends on compiler implementation and your structure fields.)

#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>  // for offsetof

typedef struct
{
  int rows;
  int cols;
  double data[1];
  // or with C99: double data[];
} Matrix;

Matrix *initMatrix(int rows, int cols)
{
  Matrix *ptr;
  double *data;
  ptr = (Matrix *)malloc(
     offsetof(Matrix, data) // = size of Matrix without double data[1];
     // with C99's "double data[];" or with the non-standard 
     // "double data[0];" you can replace offsetof(...) with: sizeof(Matrix)
     + sizeof(double) * rows * cols); // dynamic array size
  if(ptr == NULL) {
    perror("malloc failed");
  } else {
    // save rows and columns for later accessing the matrix
    ptr->rows = rows;
    ptr->cols = cols;

    data = ptr->data;
    for(int i = 0; i < rows; i++) {
      for(int j = 0; j < cols; j++) {
        *(data) = 0;
        data++;
      }
    }
  }

  return ptr;
}

int main(void) 
{
  Matrix *mptr = initMatrix(2, 2);
  // use matrix

  // a single free is sufficient with the approach above
  free(mptr); mptr = NULL;

  return 0;
}

Instead of incrementing the pointer data you can calculate the array index

    for(int i = 0; i < rows; i++) {
      for(int j = 0; j < cols; j++) {
        ptr->data[i * cols + j] = 0;
      }
    }

Or for the 0-initialization you can use memset

    memset(ptr->data, 0, sizeof(double) * rows * cols);

Or use calloc(1, offsetof(Matrix, data) + sizeof(double) * rows * cols); instead of malloc to get zero initialized memory.