Segmentation fault (core dumped) due to fgets - I think

Question

but I keep getting this error when I run this program. I think it's because of the fgets function. I tried initializing the input variable to NULL to see if that'll help, but it didn't. I also have a hunch that I might need to malloc to solve the problem. But your help is highly appreciated.

int main(int argc, char* argv[])
{
    char* input = NULL;

    // ensure one and only one command line argument
    if (argc != 2)
    {
        printf("Usage: %s [name of document]\n", argv[0]);
        return 1;
    }

    // open a new document for writing
    FILE* fp = fopen(argv[1], "w");

    // check for successful open
    if(fp == NULL)
    {
        printf("Could not create %s\n", argv[1]);
        return 2;
    }

    // get text from user and save to file
    while(true)
    {
        // get text from user
        printf("Enter a new line of text (or \"quit\"):\n");
        fgets(input, 50, stdin);

        // if user wants to quit
        if (input != NULL && strcmp(input, "quit") == 0)
        {
            free(input);
            break;
        }
        // if user wants to enter text
        else if (input != NULL)
        {
            fputs(input, fp);
            fputs("\n", fp);
            printf("CHA-CHING!\n\n");
            free(input);
        }
    }

    // close the file and end successfuly
    fclose(fp);
    return 0;
}

Answer 1

You never malloc-ed input, so yeah, fgets is dereferencing the NULL pointer as its buffer, and that's going to die. Either change input to a stack array (and remove the free for it) or actually call malloc to allocate memory so input isn't pointing to NULL.

Answer 2

While you can use malloc() here, it is not really necessary. You can #define a reasonable maximum line length, and declare a character array to hold the input. If you do this, you can remove the frees from your code.

You also have an issue with the way that you are using fgets(). The trailing \n is kept by fgets(), but your comparisons are ignoring this. Consequently, input is never equal to "quit", and is certainly never NULL. I have included some code that removes the trailing newline after reading into input; the code also clears any remaining characters from the input stream, which is possible in the event that the user enters more than MAXLINE - 1 characters. The test for text input is then simply if (input[0]). Alternatively, you could change your tests to take into account the extra '\n' character.

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

#define MAXLINE  1000

int main(int argc, char* argv[])
{
    char input[MAXLINE];
    char *ch;                       // used to remove newline
    char c;                         // used to clear input stream

    // ensure one and only one command line argument
    if (argc != 2)
    {
        printf("Usage: %s [name of document]\n", argv[0]);
        return 1;
    }

    // open a new document for writing
    FILE* fp = fopen(argv[1], "w");

    // check for successful open
    if(fp == NULL)
    {
        printf("Could not create %s\n", argv[1]);
        return 2;
    }

    // get text from user and save to file
    while(true)
    {
        // get text from user
        printf("Enter a new line of text (or \"quit\"):\n");
        fgets(input, MAXLINE, stdin);

        //  remove trailing newline
        ch = input;
        while (*ch != '\n' &&  *ch != '\0') {
            ++ch;
        }
        if (*ch) {
            *ch = '\0';
        } else {         // remove any extra characters in input stream
            while ((c = getchar()) != '\n' && c != EOF)
                continue;
        }

        // if user wants to quit
        if (strcmp(input, "quit") == 0)
        {
            break;
        }

        // if user wants to enter text
        else if (input[0])
        {
            fputs(input, fp);
            fputs("\n", fp);
            printf("CHA-CHING!\n\n");
        }
    }

    // close the file and end successfuly
    fclose(fp);
    return 0;
}

Answer 3

Their are some problems in your code.

1. You have not allocated memory to input character pointer. Hence you can't store characters in it, hence you get segmentation fault.

2. Also you are freeing more than once, which is incorrect.

So, a code, with the above modification would be something like this:

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

int main(int argc, char* argv[])
{
      char* input = malloc(sizeof(char) * 50);

      // ensure one and only one command line argument
      if (argc != 2)
      {
          printf("Usage: %s [name of document]\n", argv[0]);
          return 1;
      }

      // open a new document for writing
      FILE* fp = fopen(argv[1], "w");

      // check for successful open
      if(fp == NULL)
      {
          printf("Could not create %s\n", argv[1]);
          return 2;
      }

      // get text from user and save to file
      while(1)
      {
          // get text from user
          printf("Enter a new line of text (or \"quit\"):\n");
          fgets(input, 50, stdin);

          // if user wants to quit
          if (input != NULL && strcmp(input, "quit\n") == 0)
          {
              free(input);
              break;
          }
          // if user wants to enter text
          else if (input != NULL)
          {
              fputs(input, fp);
              fputs("\n", fp);
              printf("CHA-CHING!\n\n");
              // free(input);
          }
      }

      // close the file and end successfuly
      fclose(fp);
      return 0;
}

Hope it helps your problem. Cheers.

Answer 4

I think it's because of the fgets function.

Yes: passing NULL pointer to fgets makes no sense, isn't allowed, and will cause a crash.

I might need to malloc to solve the problem.

You need to pass a pointer to a suitable buffer for fgets to read input into. Whether that buffer is malloced, a local or a global array, is irrelevant.

TL;DR: think about what you are doing.