Re: [PATCH 6/9] reftable/stack: return stack segments directly
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 13, 2025, 11:10 UTC
- Message-ID
- <aOzeMLVx6P_AbxCQ@pks.im>
- In-Reply-To
- <CAOLa=ZSfiD4KTSMyc2WW4oC2zyDO9CHmUWwnws3GhCdbtL4rAA@mail.gmail.com>
On Mon, Oct 13, 2025 at 02:01:15AM -0700, Karthik Nayak wrote:
Show 35 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> >> diff --git a/reftable/stack.c b/reftable/stack.c
> >> index f91ce50bcd..9d9326ce0e 100644
> >> --- a/reftable/stack.c
> >> +++ b/reftable/stack.c
> >> @@ -1639,29 +1640,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
> >>
> >> REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
> >> if (!sizes)
> >> - return NULL;
> >> + return REFTABLE_OUT_OF_MEMORY_ERROR;
> >>
> >> for (size_t i = 0; i < st->merged->tables_len; i++)
> >> sizes[i] = st->tables[i]->size - overhead;
> >>
> >> - return sizes;
> >> + *seg = suggest_compaction_segment(sizes, st->merged->tables_len,
> >> + st->opts.auto_compaction_factor);
> >> + reftable_free(sizes);
> >> +
> >> + return 0;
> >> }
> >>
> >> int reftable_stack_auto_compact(struct reftable_stack *st)
> >> {
> >> struct segment seg;
> >> - uint64_t *sizes;
> >> + int err = 0;
> >
> > The initialization is unnecessary.
> >
>
> True, but I always think its nicer to define the base state like this.
> This makes it less error prone when/if the code is modified in the
> future. But doesn't really matter, so let me remove it.The counterargument to this is that it doesn't give us any nice compiler warnings if a specific return forgets to set the variable, so we may return successfully by accident. If you leave this uninitialized then the compiler warns you for every early return where you don't initialize the variable. For all I can tell this warning is also quite reliable.
Patrick