On Sat, Dec 21, 2024 at 09:36:54AM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> 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@xxxxxxxxxxxxx> > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > > --- > > 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. True. I was initially erring on the side of being overly cautious here because I didn't yet have the idea to adapt `reftable_malloc()` and `reftable_realloc()` so that they have the same behaviour as NonStop. Let me redo this change and reduce it to the minimum required one. Patrick
