Subject: Re: [PATCH 6/9] reftable/stack: return stack segments directly

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 10 Oct 2025 13:22:35 +0200

On Fri, Oct 10, 2025 at 12:27:10PM +0200, Karthik Nayak wrote:
> The `stack_table_sizes_for_compaction()` function returns individual
> sizes of each reftable table. This function is only called by
> `reftable_stack_auto_compact()` to process the table sizes.

Maybe: "to decide which tables need to be compacted, if any."

> Modify the function to directly return the segments, this avoids the

s/this/which/

> extra step of receiving the sizes only to pass it to
> `suggest_compaction_segment()`.
> 
> A future commit will also add functionality for checking if

s/if/whether/

> auto-compaction is necessary without performing it. This change allows
> code re-usability in that context.
> 
> Signed-off-by: Karthik Nayak <[email-masked]>
> ---
>  reftable/stack.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> 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.

Other than that this patch looks good to me.

Patrick