Factor our realloc handling to a separate function

[krnowak]

While working on source/reference split, I added another growable array that held the references, which meant another piece of code doing overflow checks and invoking realloc, so I decided to factor that out to a single place. While at it, I changed one size member to use zip_uint64_t.

This also contains a small behavior change in one place, please see the comment in the code.

[dillof]

Thanks, that's an excellent idea, which should make code simpler and easier to understand.

I have some suggestions to make it fit better with the conventions used by libzip, see below.

(I guess we should start a style guide.)

[dillof]

Error propagation in libzip is done via a zip_error_t *error argument, usually the last argument of the function. The function fills in error via zip_error_set() and signals that an error occurred via a special return value (often -1 or false).

Also, please put the return type and function prototype on the same line. (This is our new style; we haven't converted all of libzip to this new style yet.)

[krnowak]

Error propagation in libzip is done via a zip_error_t *error argument, usually the last argument of the function. The function fills in error via zip_error_set() and signals that an error occurred via a special return value (often -1 or false).

This made the calling code even simpler, thanks.

Also, please put the return type and function prototype on the same line. (This is our new style; we haven't converted all of libzip to this new style yet.)

Alright, done.

[dillof]

Please add {/} around single line blocks.

Why do you special case 0 here? If no additional items are required, I would expect this function to do nothing.

[krnowak]

Honestly? I don't remember. :) I wrote this function back in July or August, when working on that streams branch. Just borrowed that code from there. I probably should have documented this case. Anyway, will add the early return here instead.

[krnowak]

Done.

[dillof]

We usually do overflow checks for the addition of two unsigned values as if (old_allocated + additional_elements < old_allocated), which does not require knowing the type of the elements.

I would prefer uniformity in our overflow checks so they are easy to recognise as such.

[krnowak]

Done. I kept the multiplication overflow check as is, though.

[dillof]

Please use a different name for macro and function (like zip_realloc_implementation for the function).

Also, since some C compilers warn about symbols beginning with _ as being reserved, please don't use that prefix for new internal symbols.

[krnowak]

Done.

[krnowak]

I was thinking about dropping the element_size parameter from the macro and make the macro to pass (sizeof(**(inout_memory))) to zip_realloc_implementation instead, but this is probably too much magic.

[krnowak]

Not sure if this fiddling with max is necessary, please let me know what you think. In general switching between size_t and uint64 seems to be icky, especially on Windows platforms. Waiting for Appveyor to finish its work to see if msvc has some complaints about the code.

[dillof]

You can always use SIZE_MAX here: new_allocated * element_size must fit in a size_t.

[krnowak]

Done.