I'am getting a Segmentation fault while using fgetc(). How can I fix it?

Question

I want to read from a filestream for z bytes. Then I want to give the value in a char array back. I would appreciate it if you could give me an explaination. That's my code since now:

char * getData(FILE * fp, long * x)
{
  int z = 0;
  char * data = malloc(sizeof(char) * BUFFLENGTH);
  strcpy(data,"");

  while(z < BUFFLENGTH-2)
  {
    if(feof(fp) == 0)
    {
      data[z] = fgetc(fp);
      z++;
      x++;
    }
    else
    {
       strcat(data,"\0");
       return data;
    }
  }    
}

I know that the segmentation fault is triggered threw this line:

data[z] = fgetc(fp);

But I dont know why.

Possible duplicate of [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — too honest for this site, Mar 05 '16 at 14:21
`strcat(data,"\0");` is intended to append zero bytes, but will cause *undefined behavior* because what is not a pointer to null-terminated string may be passed as the first argument. — MikeCAT, Mar 05 '16 at 14:21
Do you *really know* the line triggering segmentation fault, using debugger or something? You didn't post your imagination, did you? — MikeCAT, Mar 05 '16 at 14:23
@MikeCAT ... but still depends on the contents of `data[]` ... BTW: `count` is loop invariant and `*x` is never referenced. — wildplasser, Mar 05 '16 at 14:24
@MikeCAT I check this in an other function. Its not NULL. I thought '\0' is one byte large? That's why you have to make a char array one char larger as you need, or am I wrong? — missjohanna, Mar 05 '16 at 14:26
`"\0"` is two-byte large because it contains `'\0'` that is written as escape sequence and `'\0'` that is automatically added. And it means to append zero bytes because `'\0'` means termination of string and it comes as the first bytes. Use `data[z] = '\0';` to terminate the string, — MikeCAT, Mar 05 '16 at 14:27
Other note: `count` is not updated in the loop, so `while(count < BUFFLENGTH-2)` means `while(1)` if `BUFFLENGTH` is a constant value larger than 2. — MikeCAT, Mar 05 '16 at 14:29

score 0 · Accepted Answer · answered Mar 05 '16 at 14:36

0

You should check if the allocation was successful.
You should check if the reading was successful after calling fgetc() instead of using feof().
This usage of strcat() will invoke undefined behavior if one or more bytes are read from the file and no byte its value is zero is read because a pointer pointing something which is not null-terminated string will be passed to strcat() and it will access area allocated via malloc() and not initialized. Instead of this, you should terminate the string by adding '\0'.
You should return something explicitly even if the length read exceeds the limit, otherwise you will invoke undefined behavior if the caller use the return value.

code with these advices applied:

char * getData(FILE * fp, long * x)
{
  int z = 0;
  char * data = malloc(sizeof(char) * BUFFLENGTH);
  if(data == NULL) return NULL;
  /* strcpy() isn't needed because it will be overwritten */

  while(z < BUFFLENGTH-2)
  {
    int input = fgetc(fp);
    if(input != EOF)
    {
      data[z] = input;
      z++;
      x++;
    }
    else
    {
       data[z] = '\0';
       return data;
    }
  }
  free(data);
  return NULL;
}

answered Mar 05 '16 at 14:36

MikeCAT

73,922
11
45
70

I tried to access it with: char * new = getData(fp, &byteatm); printf("%s", new); but it always says (null). If you free(data) after the while loop isnt it destroyed in memory? – missjohanna Mar 05 '16 at 14:46
`free()` will destroy the (validity of) contents in memory. Make the file size below the limit or change the process for files that are too large. – MikeCAT Mar 05 '16 at 14:47
It's just one file with 1042 Bytes. How can such a small file be too large? – missjohanna Mar 05 '16 at 15:16
It depends on the value of `BUFFLENGTH`. For example, if its value is `80`, any file that is 78 bytes or larger are too large. – MikeCAT Mar 05 '16 at 15:17
But if EOF is reached it returns data and leaves the function, or? – missjohanna Mar 05 '16 at 15:33
[Coudl't reproduce](http://melpon.org/wandbox/permlink/KSMhakVHzfQOMkGs) that `NULL` is returned. – MikeCAT Mar 06 '16 at 00:11
If I use your tool. I cant neither. Even if I add my whole file (file I want to open). But if I say FILE * fp = fopen(myfile,"r"); It always return NULL. – missjohanna Mar 06 '16 at 17:28
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/105498/discussion-between-missjohanna-and-mikecat). – missjohanna Mar 06 '16 at 17:53
Couldn't reproduce in neither my local environment (WIndows 7 x64, GCC 4.8.1) nor [online](http://melpon.org/wandbox/permlink/FKNJMRR6wgSNTVwE). – MikeCAT Mar 06 '16 at 23:25
I think I found the mistake. If z< BUFFLENGTH and EOF is not reached it gave simply NULL back. But if I add a return data; after the while loop its working. – missjohanna Mar 07 '16 at 04:18

I'am getting a Segmentation fault while using fgetc(). How can I fix it?

1 Answers1