I keep getting a seg fault not sure why-CodePudding

This code is to make and print a matrix out but i am not sure why i am getting seg fault, it is because I am not freeing memory, if so how would i free it?

void printMatrix(struct Matrix *M){
  struct Matrix *temp = M;
  for(int i=0; i< temp->rows;i  ){
    for(int j=0; j< temp->cols;j  ){
      printf("%.f",getVal(temp,i,j));
    }
    printf("\n");
  }
}
void makeMatrix(struct Matrix *M, int row, int col, int num){
   M = malloc(sizeof(struct Matrix));
  M->rows=row;
  M->cols=col;
  M->m =malloc(100*sizeof(double)*M->rows*M->cols);
  for(int i=0; i<M->rows;i  ){
    for(int j=0; j<M->cols;j  ){
        setVal(M,i,j,num);
    }
  }
  free(M);
}
int  main(int argc, char const *argv[]) {
  struct Matrix *test;
  makeMatrix(test,10,10,10);
  printMatrix(test);

  return 0;
}

CodePudding user response：

First, always have to check if the malloc successfully allocates memory. So after the first call to malloc, you should write something like that:


    if(!M)
    {
        printf("malloc failed to allocate memory for M");
        return;
    }

and so on. Also you should free each memory space you allocated with malloc. In your case you should also free(M->m)

CodePudding user response：

Your makeMatrix function is wrong. Parameter M is a local variable when makeMatrix executed. Thereofre any changes to M are not visible when the function ends. As result test is not initialized when passed to printMatrix causing failure then the pointer is dereferenced.

The solution is returning M from the function by value.

struct Matrix *makeMatrix(int row, int col, int num){
  struct Matrix *M = malloc(sizeof(struct Matrix));
  if (!M) return NULL;
  M->rows=row;
  M->cols=col;
  M->m =malloc(100*sizeof(double)*M->rows*M->cols);
  if (!M->m) {
    free(M);
    return NULL;
  }
  for(int i=0; i<M->rows;i  ){
    for(int j=0; j<M->cols;j  ){
        setVal(M,i,j,num);
    }
  }
  return M;
}

Usage:

struct Matrix *test = makeMatrix(10,10,10);

Moreover, malloc(100*sizeof(double)*M->rows*M->cols); looks a bit wasteful because it consumes 100x more memory than needed. I'm pretty sure that malloc(sizeof(double)*M->rows*M->cols); would suffice.