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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 21, 2024, 17:36 UTC
Message-ID: <xmqq34ihc4zt.fsf@gitster.g>
In-Reply-To: <20241221-b4-pks-reftable-oom-fix-without-readers-v1-3-12db83a3267c@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 16 quoted lines

> 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.