If Realloc() fails, does the caller lose the data prom the previous malloc() call?

Viewed 112

The following is a quote from an exam (1% top university).

I failed, because my answer differed from the "approved" answer.

I have a hunch that his (professor, known expert in C) answer is not correct.

The following, is the question followed by the "approved" answer.

There is a potential bug in the following function. What is it and how would I fix it?

Hint: this has something to do with the use of the realloc() function. Please identify the line numbers you would change and what you would replace them with.

BOOLEAN lengthen_string(char* string, const char newcontents[])
{
        int newlen = strlen(string) + strlen(newcontents) + 1;
        string = realloc(string, newlen);

        if (!string) {
                perror("malloc");
                return FALSE;
        }

        strcat(string, newcontents);
        return TRUE;
}

The "correct" answer provided by the professor was:

line 4: realloc returns a NULL pointer when it fails to allocate. This means that on failure the original data is lost.

To fix this, assign the result of realloc to a temporary variable and test that first.

Ie: line 4:

char * temp=realloc(string, newlen);
if(!temp) ... (all remains the same)

after old line 9, string = temp;

Any thoughts?

BTW, my answer was that @string is a local variable, and the prototype of the function should be char **string, where the caller passes a pointer to its string pointer, and then the callee would assign any realloc() return value to *string

Any thoughts?

1 Answers

You are both correct.

The professor is correct, in that realloc() does not alter the passed-in memory on failure, thus leaving the input string pointer intact, but if the NULL return value on failure is assigned immediately to string then the original data is lost and leaked. So a check for failure first is needed before assigning the new pointer value to string.

You are correct, in that string needs to be passed by pointer so it can be re-assigned a new value if realloc() returns a different memory address.

The correct solution would look more like this:

BOOLEAN lengthen_string(char** string, const char newcontents[])
{
    if (!string)
    {
        errno = EINVAL;
        perror("bad input");
        return FALSE;
    }
    size_t newsize = strlen(*string) + strlen(newcontents) + 1;
    char *temp = realloc(*string, newsize);
    if (!temp)
    {
        perror("realloc failed");
        return FALSE;
    }
    strcat(temp, newcontents);
    *string = temp;
    return TRUE;
}

Alternatively, there is some room for optimization, eg:

BOOLEAN lengthen_string(char** string, const char newcontents[])
{
    if (!string)
    {
        errno = EINVAL;
        perror("bad input");
        return FALSE;
    }
    char *temp;
    if (!*string)
    {
        temp = strdup(newcontents);
        if (!temp)
        {
            perror("strdup failed");
            return FALSE;
        }
    }
    else
    {
        size_t offset = strlen(*string);
        size_t size = strlen(newcontents) + 1;
        temp = realloc(*string, offset + size);
        if (!temp)
        {
            perror("realloc failed");
            return FALSE;
        }
        memcpy(temp + offset, newcontents, size);
    }
    *string = temp;
    return TRUE;
}
Related