Search code examples
cstringsegmentation-faultstrlencs50

C string from GetString() when strlen produces segmentation fault


I am running a program in C. When I run the program I get a segmentation fault error. IN gdb when I backtrace it tells me

Program received signal SIGSEGV, Segmentation fault. __strlen_sse2_bsf () at ../sysdeps/i386/i686/multiarch/strlen-sse2-bsf.S:51 51 movdqu (%edi), %xmm1

I believe it has to do with strlen.

The only time I use strlen is:

    string s = GetString();

    int stringlength = strlen(s);

When I change strlen for sizeof error stops.

What is wrong with my code?

Documentation of GetString

/*
 * Reads a line of text from standard input and returns it as a 
 * string (char *), sans trailing newline character.  (Ergo, if
 * user inputs only "\n", returns "" not NULL.)  Returns NULL
 * upon error or no input whatsoever (i.e., just EOF).  Leading
 * and trailing whitespace is not ignored.  Stores string on heap
 * (via malloc); memory must be freed by caller to avoid leak.
 */

string GetString(void) {
    // growable buffer for chars
    string buffer = NULL;

    // capacity of buffer
    unsigned int capacity = 0;

    // number of chars actually in buffer
    unsigned int n = 0;

    // character read or EOF
    int c;

    // iteratively get chars from standard input
    while ((c = fgetc(stdin)) != '\n' && c != EOF)
    {
        // grow buffer if necessary
        if (n + 1 > capacity)
        {
            // determine new capacity: start at 32 then double
            if (capacity == 0)
                capacity = 32;
            else if (capacity <= (UINT_MAX / 2))
                capacity *= 2;
            else
            {
                free(buffer);
                return NULL;
            }

            // extend buffer's capacity
            string temp = realloc(buffer, capacity * sizeof(char));
            if (temp == NULL)
            {
                free(buffer);
                return NULL;
            }
            buffer = temp;
        }

        // append current character to buffer
        buffer[n++] = c;
    }

    // return NULL if user provided no input
    if (n == 0 && c == EOF)
        return NULL;

    // minimize buffer
    string minimal = malloc((n + 1) * sizeof(char));
    strncpy(minimal, buffer, n);
    free(buffer);

    // terminate string
    minimal[n] = '\0';

    // return string
    return minimal;
}

Solution

  • The description of the getString() function clearly states that it can return NULL on an error or on EOF.

    If you pass the return value to strlen() without checking, your program will crash.

    string s = GetString();
    int stringlength = 0;
    
    if (s != 0)
        stringlength = strlen(s);
    

    This at least won't crash.

    You might also notice how much confusion the typedef char *string; causes and how little benefit it confers, and take it to heart. You don't have to repeat the mistakes of those who are teaching you.

    I also observe that the code fragment:

    // minimize buffer
    string minimal = malloc((n + 1) * sizeof(char));
    strncpy(minimal, buffer, n);
    free(buffer);
    

    could be better, and more simply, written as:

    string minimal = realloc(buffer, n + 1);
    

    to shrink the allocation to the correct size.