Re: [PATCH 3/4] reftable/stack: fix zero-sized allocation when there are no readers

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Similar as the preceding commit, we may try to do a zero-sized
> allocation when reloading a reftable stack that ain't got any tables.
> It is implementation-defined whether malloc(3p) returns a NULL pointer
> in that case or a zero-sized object. In case it does return a NULL
> pointer though it causes us to think we have run into an out-of-memory
> situation, and thus we return an error.
>
> Fix this by only allocating arrays when they have at least one entry.
> Refactor the code so that we don't try to access those arrays in case
> they are empty.
>
> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  reftable/stack.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)

This somehow did not cleanly apply, so I whiggled it in manually.

I probably wouldn't mixed the "size_t i" changes into this fix if I
were doing it.  To avoid "while (*names)" loop, I would have made it
to "for (size_t j = 0; j < names_len; j++)" and kept the existing
use of "i" intact, instead.  And reintroducing for() scoped "i"
three times did not seem to make it easier to read the result.

I am not convinced we need to avoid "while (*names)", by the way.
If "names" were NULL, names_length() would already have segfaulted
anyway, and basics.c:parse_names(), when not returning NULL (which
would have been caught by the sole caller of reload_once() as an
error), makes sure it gives its caller a NULL-terminated array.

But other than that, this seems to make sure that we avoid
unnecessary work when cur_len or new_readers is zero and avoids
asking for 0-sized allocation as a side effect of doing so, which is
good.

Will queue.  Thanks.