CODEWITHSUNDEEP
carraysmultidimensional-arraysegmentation-fault
MEURSAULT
MEURSAULT

Reputation: 8819

Why am I getting a seg fault (multidimensional arrays C)

I can't seem to figure out what I'm doing wrong.

int main(int argc, char **argv) {
  int height = 4, width = 6;
  int **map;
  map = (int **)(malloc(height * sizeof(int*)));
  for(i = 0; i < height; i++) {
    map[i] = (int *)(malloc(width * sizeof(int)));
  }
  fill_map(&map, height, width);
}

void fill_map(int ***map, int height, int width) {
  int i, k, character;
  for(i = 0; i < height; i++) {
    k = 0;
    while((character = getchar()) != '\n') {
        *map[i][k] = character;
        k++;
    }
  }
}

I get a segfault in fill_map, in the inner while loop, why?

Upvotes: 0

Views: 121

Answers (6)

John Bode
John Bode

Reputation: 123478

Your segmentation fault is coming from the fact that you're dereferencing map incorrectly; you should write it as (*map)[i][k] instead of *map[i][k]. Although it's better to leave off the & in the call to fill_map and adjust the prototype accordingly:

fill_map(map, height, width);
...
void fill_map(int **map, int height, int width)
{
  ...
  map[i][k] = character;
  ...
}

You can clean up your malloc calls like so:

map = malloc(height * sizeof *map);

and

map[i] = malloc(width * sizeof *map[i]);

The casts aren't necessary in C1, and are considered bad practice.

1 This is not true in C++; a cast is required, but if you're writing C++ you should be using new instead of malloc.

Upvotes: 0

Jens Gustedt
Jens Gustedt

Reputation: 78923

What your are using is not a 2D array but just a simulation of it. In modern C, starting with C99, with "variably modified types" your task is as simple as this:

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

void fill_map(size_t height, size_t width, char map[height][width]) {
  int character;
  for(size_t i = 0; i < height; i++) {
    for (size_t k = 0; k < width; k++) {
      if ((character = getchar()) != '\n')
        map[i][k] = character;
      else break;
    }
  }
}

int main(int argc, char **argv) {
  size_t height = 4, width = 6;
  char (*map)[width] = malloc(sizeof(char[height][width]));
  fill_map(height, width, map);
}
  • pass the matrix dimensions first, then you can simply use them in the declaration of the matrix parameter
  • use size_t for indexing
  • don't cast the return of malloc
  • be careful with the bounds of your loop
  • also better check end of file (homework)

Upvotes: 0

perreal
perreal

Reputation: 97958

Don't send &map to the function, change the prototype to receive (int **map) or, use (*map)[i][k]. This is because indirection operator * has lower precedence than [] operator.

Upvotes: 3

Happy Green Kid Naps
Happy Green Kid Naps

Reputation: 1673

First -- heed advice given by 'prmg' and 'perreal'.

Do not cast the return value of malloc. malloc returns a void * and the cast is unncessary. Should you forget to #include <stdlib.h>, you are setting yourself up for trouble.

map = malloc( height * sizeof *map );

Upvotes: 0

dbeer
dbeer

Reputation: 7203

I would use gdb or another debugger. Compile the program with debug symbols, run it in there, and it will tell you the line on which you're segfaulting.

Upvotes: 0

Masa
Masa

Reputation: 311

How do you expect to end your while loop, did you ever kept a "\n" im your array? You have just allocated memory and trying to travel over it. It never finds an "\n" , thus while loop goes out the memory location and you get the segmentation fault.

Upvotes: 1

Related Questions