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.

Answer 1

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