Why does accessing my matrix give me a seg fault?

Question

When ever I try to access my matrix which is Line*** I get a seg fault. A simple operation such as (matrix[i][j]->vbit == 00),{int vbit} gives me a segmentation fault. I am assuming it is within the constructor, but I can't seem to find the issue. Anyone see it?

Line ***getSets(int width, int height){
int i;
int j;
Line *temp;
Line line;

printf("Make %d sets with %d lines\n",height,width);

Line*** matrix = (Line***)calloc(height,sizeof(Line**));


for(i=0;i < height;i++){
    matrix[i] = (Line**)calloc(width,sizeof(Line*));
}    

/// Set all vbits to 0
for(i = 0; i < height;i++){
    for(j = 0;j <width; j++){
        temp = matrix[i][j];
        temp = malloc(sizeof(Line));
        temp->vbit = 0;        
        temp->tag = 0;
        temp->lastUsed = 0;
    }
}
return matrix;}

Do not post your code as image. Please post your code as text directly in your question. — MikeCAT, May 03 '16 at 02:56
You didn't assign the allocated buffer to `matrix[i][j]` and it is uninitialized. — MikeCAT, May 03 '16 at 02:58
Don't post images of code, post the *actual* code. That having been said, your problem is obvious: In your inner loop, you're assigning to `temp` from a matrix entry, then immediately reassigning to `temp`, rendering the first assignment useless. Instead, you need to assign *from* `temp` *to* the matrix entry after you've allocated storage for it. — Tom Karzes, May 03 '16 at 02:59
`temp = matrix[i][j];temp = malloc(sizeof(Line));` --> `temp = matrix[i][j] = malloc(sizeof(Line));` — BLUEPIXY, May 03 '16 at 02:59
Apologies for the formatting, this one my first post on here and I was unfamiliar with the formatting. Will follow in the future. — John, May 03 '16 at 03:08

score 2 · Accepted Answer · answered May 03 '16 at 02:59

You only ever allocate to temp, not your actual matrix element:

temp = matrix[i][j];
temp = malloc(sizeof(Line));

Do this instead:

matrix[i][j] = malloc(sizeof(Line));
temp = matrix[i][j];

Or

for(j=0; j<width; j++) {
    temp = malloc(sizeof(Line));
    memset(temp, 0, sizeof(Line));
    matrix[i][j] = temp;
}

Plus, you should really really be checking the result of calloc and malloc.

score 0 · Answer 2 · edited May 23 '17 at 12:16

As a rule of thumb: whenever you think you need more than 2 levels of indirection, it always means that your program design is fundamentally broken.

In this case, you came up with the 3 levels of indirection just because you don't allocate multi-dimensional arrays correctly. You should do that like this.

That being said, even if you would allocate dynamic memory correctly, the root of your problems is still the program design. Consider doing something completely different. For example, your program says that it is making x sets of y lines. So why doesn't it? Create a set class. Here is an example of an alternative design which you can expand with further functionality:

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

// assuming: 1 set has several lines, 1 line has several coordinates

typedef struct
{
  // whatever you want in here, doesn't matter
  int x;
  int y;
} line_t;

typedef struct
{
  size_t  lines_n;
  line_t* line;
} set_t;


bool get_sets (size_t height, size_t width, set_t set[height])
{
  for(size_t i=0; i<height; i++)
  {
    set[i].lines_n = width;
    set[i].line = malloc( sizeof(line_t[width]) );
    if(set[i].line == NULL)
    {
      return false;
    }

    for(size_t j=0; j<width; j++)
    {
     // initialize all members:
      set[i].line[j].x = 0;
      set[i].line[j].y = 0;
    }
  }

  return true;
}

int main (void)
{
  int height = 3;
  int width = 5;
  set_t set [height];

  printf("Make %d sets with %d lines\n", height, width);
  bool result = get_sets(height, width, set);
  if(result == false)
  {
    // out of memory error
  }  

  return 0;
}

Why does accessing my matrix give me a seg fault?

2 Answers2