Ex34: possible bug in `DArray_resize()`

C
#1

From page 224 of the book I found the following code:

static inline int DArray_resize(DArray * array, size_t newsize)
{
    array->max = newsize;
    check(array->max > 0, "The newsize must be > 0.");

    void *contents = realloc(array->contents, array->max * sizeof(void *));
    // check contents and assume realloc doesn't harm the original on error

    check_mem(contents);

    array->contents = contents;
    return 0;
error:
    return -1;
}

The problem with this code is that it changes array->max before assuming nothing is changed on error (memory error). If an error does occur, this assumption is wrong, because array->max was already overwritten. The next access to the array will receive the new value for array->max.

So, if newsize is larger than the original array->max the next access might produce weird results.

The solution is to only change array->max once we are certain no error occurred during reallocate.

#2

No, because the check_mem(contents) will error out, and jump to error:, returning -1. If someone tries to use it after that then they’re violating the rules. If you wanted to prevent that, then what you’d actually do is clear out or negate array->max in the error: handling code before returning -1.

#3

Actually I was inclined to code it this way, bypassing any misuse and not changing the input until I can safely do so:

static inline int DArray_resize(DArray * array, size_t newsize)
{
    check(newsize > 0, "The newsize must be > 0.");

    void *contents = realloc(array->contents, array->max * sizeof(void *));
    // check contents and assume realloc doesn't harm the original on error

    check_mem(contents);

    array->max = newsize;
    array->contents = contents;
    return 0;
error:
    return -1;
}

Kind regards, Guus.

#4

Yep, you can do that too. Also, if you wanted to really be pedantic you’d set array->max = -1 and array->contents = NULL int he error: handler so people couldn’t use it at all.