I keep getting a seg fault not sure why

Question

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;
}

`test` has not been assigned a value, it has garbage, you cannot print it.. Remember: in C all arguments are passed by value (*what happens in the other function, stays in the other function*) — pmg, Oct 25 '21 at 07:15
Remember that in C arguments are passed *by value*. That means the value in the call is copied into the functions local argument variable. Any modifications to the argument variable (like assigning to it) will only happen to the local argument variable. Either *return* the new value or do some research about *emulating pass by reference in C*. — Some programmer dude, Oct 25 '21 at 07:19
Also be careful when and where you delete the matrix. And make sure that *all* `malloc` calls have a corresponding call to `free` (which your code doesn't have). — Some programmer dude, Oct 25 '21 at 07:20
And in `100*sizeof(double)*M->rows*M->cols`, why multiply with `100`? — Some programmer dude, Oct 25 '21 at 07:21
There are some definitions missing from your question but perhaps [this](https://godbolt.org/z/jrE53dnTK) would be a more complete approach. — Ted Lyngmo, Oct 25 '21 at 07:32
Main issue is found in the linked duplicate. But obviously it doesn't make any sense to `free(M)` from the allocation function either. — Lundin, Oct 25 '21 at 10:45

score 1 · Accepted Answer · answered Oct 25 '21 at 08:01

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.

so what does the if statement do this solved my problem but I am not sure what is the point of the if(), also just for clarification when I make a pointer as a paramter the life span is only within the paramter right ? — Shehab A, Oct 25 '21 at 08:22
@ShehabA, because `malloc` may fail. The validity of the returned pointer is checked by `!M` ("not" + pointer). The lifespan of the parameter ends when `return` is reached. — tstanisl, Oct 25 '21 at 08:27

score 0 · Answer 2 · answered Oct 25 '21 at 07:44

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)

I keep getting a seg fault not sure why

2 Answers2