Should We Free Memory Allocated By GetString()?

Question

Running valgrind with line 42 commented out shows that there are indeed some leaks. If we DON'T comment out line 42, then there are no leaks.

Here is the code

// copy-1.mp4 from Week4 Walkthroughs
// copied using random access into string array

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

int main(void)
{
    printf("Enter string: ");
    char* s = GetString();
    if (s == NULL)
    {
        return 1;
    }

    char* t = malloc((strlen(s) + 1) * sizeof(char));
    if (t == NULL)
    {
        free(s);
        return 1;
    }

    // so that i iterate up to and including the null character
    for (int i = 0, n = strlen(s); i <= n; i++)
    {
        t[i] = s[i];
    }

    printf("Capitalizing copy...\n");
    if (strlen(t) > 0)
    {
        t[0] = toupper(t[0]); 
    }

    printf("Original string: %s\n", s);
    printf("Copy of string: %s\n", t);

    free(t);
    // free(s); // why didn't sir include this line too??

    return 0;
}

Answer 1

Short answer: yes!

Long answer: looking at the code of GetString()

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

It turns out that GetString() allocates memory for the string it gets and returns to us. And indeed it doesn't free that memory before it returns so that the string it gets stays in memory and we can use it!

This means that we're mallocing a block of memory for s (line: 13 in your code) via GetString() and we're mallocing another block of memory for t (line: 19 in your code).

And since we should free all the memory that we've allocated so far, we must free both, the memory allocated for s and the memory allocated for t by

free(s);

and

free(t);

respectively.

Now, why the professor didn't care about that? Well, as far as I remember, he mentioned something about GetString() and that it allocates memory somewhere so we should free that memory, but probably he didn't care about that in this walkthrough for demonstration and simplicity purposes.