Error in struct and 2d arrays in C

Question

#include<stdio.h>
#include<stdlib.h>
typedef struct Room {
    int values[4][4];
    int size;
} Room;

int createRoom(Room *pm, char *mazefilename)
{

    FILE *input;
    FILE *output;
    pm = (Room *)malloc(sizeof(Room *));
    input = fopen(mazefilename, "r");

    int a = 0;
    int i = 0, j = 0;
    int count;

    char  line[6];


    fscanf(input, "%s", line);
    while (!feof(input)) {
        printf("Line is  %s\n", line);
        for (i = 0; i < 4; i++) {
            int c = line[i] - '0';

            pm->values[a][i] = c;
        }
        a++;
        fscanf(input, "%s", line);
    }
    fclose(input);
    return a;
}

This is giving me segmentation fault-

Line is  1001
Line is  1001
Line is  1101
Segmentation fault (core dumped)

Why is it giving me segmentation fault.If I just print the lines instead of pm->values[a][i]=c;,it gives me the correct output.So I think something is wrong with this line-pm->values[a][i]=c;

My input file is-

score 2 · Answer 1 · edited Feb 17 '15 at 15:39

2

Your function signature and malloc call are incorrect for what you are trying to do. If you want to allocate space for the passed in pm pointer you need to pass that pointer's address:

int createRoom(Room ** pm,char *mazefilename)

And then your malloc() call has to allocate enough space for a Room structure, not a Room pointer:

*pm=malloc(sizeof(Room));

edited Feb 17 '15 at 15:39

Iharob Al Asimi

52,653
6
59
97

answered Feb 17 '15 at 15:29

mshildt

8,782
3
34
41

score 1 · Accepted Answer · edited May 23 '17 at 12:28

You are not allocating enough space

pm=(Room *)malloc(sizeof(Room *));

allocates one pointer, you need

pm = malloc(sizeof(Room));

Other issues with your code

Don't cast malloc().
while (!feof(file)) is always wrong.
You don't check the return value either of malloc() or fopen(), you must check that if you want your code to be robust.

The allocated pointer, will not be valid outside of the function, you have three options

Return the pointer and pass a as a pointer, so you can update the counter inside the function.
Pass pm as a pointer to pointer, i.e. Room **pm and pass it's address, you must then use the dereference operator to modify pm something like
```
*pm = malloc(sizeof(Room));
```

This

Room * createRoom(char *mazefilename)
{

   Room * pm;
   FILE * input;
   FILE * output;

   pm = malloc(sizeof(Room));
   if (pm == NULL)
     return NULL;

   input = fopen(mazefilename,"r");
   if (input == NULL)
   {
     free(pm);
     return NULL;
   }

   int i = 0, j = 0;
   int count;

   char  line[6];

   pm->size = 0;
   while (fscanf(input,"%s",line) == 1)
   {
       printf("Line is  %s\n", line);
       for (i = 0 ; i < 4 ; i++)
       {
         int c = line[i] - '0';
         pm->values[a][i] = c;
       }
       pm->size++;
    }
    fclose(input);
    return pm;
}

this third option, seems to be the way you intended to do it.

Thanks a lot it worked.Can you tell me why is `while (!feof(file))` wrong,it seems to be working in my case? — , Feb 17 '15 at 15:34
Just changing this `pm = malloc(sizeof(Room));` worked for me . Also if suppose I wanted to use dynamic array in my struct say `int ** values`,then how can I allocate memory for such double pointers? — , Feb 17 '15 at 15:39
Please read the link about `while (!feof(file))` and check how I fixed it in the , and yes the main problem is you was allocating space for a pointer, instead of doing it for a `struct`. — Iharob Al Asimi, Feb 17 '15 at 15:52

Error in struct and 2d arrays in C

2 Answers2