Segmentation fault caused by fgets() in C

Question

This is a problem I don't understand - I am using fgets() in main and it works. I use it (I think) in exactly the same way in a function and I get an error [Segmentation fault core dumped -- exit code 139).

This code is based on a sample program in book Ivor Horton's "Beginning C" (it's a old tile but I'm just wanting to learn the basics from it).

My program is as follows. I am working on *nix using Geany (basically, compiling with GCC). You can see that fgets works in main (output is the string you enter). But it doesn't work in the function str_in(). It gets as far as the second printf() statement to enter a string, no further. Note that in the book, Horton uses gets(). I am trying to implement a safer string input function here, but no joy.

By the way the program is supposed to sort strings stored in an array of string pointers.

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

#define TRUE 1
#define FALSE 0
#define MAX_NUM_STRINGS 50


int str_in(char **);              /*Pointer to a string pointer*/
void str_sort(char *[], int n);   /*Array of pointers to strings, number of strings in array*/
void str_out (char *[], int n);   /*Array of pointers to strings, number of strings in array*/


int main(){

    char *pS[MAX_NUM_STRINGS] = { NULL };                  /*Array of pointers to strings stored in str_space*/
    int numStrings = 0;                                     /*Count of strings*/
    char buffer[BUFSIZ];

    printf("Enter a string\n");
    fgets(buffer, BUFSIZ, stdin);
    printf("%s", buffer);
    printf("fgets works here\n\n");


    /* get string input from user - a pointer to each string is saved in pS */
    while ( str_in(&pS[numStrings]) && numStrings < MAX_NUM_STRINGS)
        numStrings++;
    if ( numStrings > 0 ){
        str_sort(pS, numStrings);
        str_out(pS, numStrings);
    }

    return 0;
}


    int str_in(char** pString){

        char buffer[BUFSIZ];
        char *p;

        printf ("Enter string:\n");

        fgets(buffer, 60, stdin);
        printf("fgets doesn't work here!!\n");

        if( buffer != NULL ){
            printf("here");
            if ((p = strchr(buffer, '\n')) != NULL)
                *p = '\0';                                  /*replace newline with null character*/
            else
                return FALSE;
            if ( strlen(buffer) > 0 ){
                strcpy(*pString, buffer);
                return TRUE;
            }
            else
                return FALSE;                               /*blank line - end of input*/
        }
        else
            return FALSE;

    }


    void str_sort(char* pStrings[], int n){
    /*sort strings by manipulating array of string pointers*/

        char *temp;
        int sorted = FALSE;
        int i = 0;

        while (!sorted){

            sorted = TRUE;
            for(i = 0; i < n - 1; i++){
                temp = pStrings[i];
                if ( strcmp(temp, pStrings[i+1]) > 1 ){
                    pStrings[i] = pStrings[i+1];
                    pStrings[i+1] = temp;
                    sorted = FALSE;
                    break;
                }
            }

        }

    }


    void str_out(char* pStrings[], int n){
    /*print strings to standard output.  Free memory as each string is printed */

        int i = 0;

        printf("Sorted strings:\n");
        for(i = 0; i < n; i++){
            printf("%s", pStrings[i]);
            free(pStrings[i]);
        }

    }

Answer 1

The segmentation fault is not caused by fgets(), but by strcpy():

strcpy(*pString, buffer);

You try to write to *pString, but you never allocate memory for it. pS in main() is just an array of null pointers.

Another thing is with the test with if( buffer != NULL ), that would never be true since buffer is an array, not a pointer.

Answer 2

You must check for the return value of fgets to see if you have successfully received something, if not then you should never use your buffer as a string as you are not NUL terminating the buffer.

/* Checking for buffer != NULL is of no use */
/* as buffer will always be not NULL since */
/* since you have allocated it as char buffer[BUFSIZ] */

if (fgets(buffer, BUFSIZ, stdin) == NULL) {
   /* buffer may not be a valid string */
}

So what you can do it to initialize the buffer to a NUL string, as soon as you enter the function (after your declarations are done

buffer[0] = 0; /* initialize to NUL string */

now you can use buffer as a string anywhere.

Also note than if BUFSIZ is too big greater than a couple of KB, then your might get seg fault due to stack overflow. If they are too big you could make buffer as "static char" instead of "char".