Re: [PATCH 04/11] reftable/stack: guard against NULL list_file in stack_destroy

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

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When reftable_new_stack() fails partway through initialization
> (e.g., reftable_buf_addstr returns an OOM error before
> reftable_buf_detach assigns p->list_file), it jumps to the error
> path which calls reftable_stack_destroy(p). At that point,
> p->list_file is still NULL because the detach never happened.
>
> reftable_stack_destroy() passes st->list_file unconditionally to
> read_lines(), which calls open(filename, O_RDONLY). Passing NULL
> to open() is undefined behavior and will typically crash.
>
> Guard the read_lines() call with a NULL check on st->list_file.
> When list_file is NULL, there are no table files to clean up
> anyway, so skipping read_lines is the correct behavior.

Nice spotting and recovery.  Well done.

>
> Pointed out by Coverity.
>
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  reftable/stack.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/reftable/stack.c b/reftable/stack.c
> index 1fba96ddb3..3fc3c0b2d1 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -171,7 +171,8 @@ void reftable_stack_destroy(struct reftable_stack *st)
>  		st->merged = NULL;
>  	}
>  
> -	err = read_lines(st->list_file, &names);
> +	if (st->list_file)
> +		err = read_lines(st->list_file, &names);
>  	if (err < 0) {
>  		REFTABLE_FREE_AND_NULL(names);
>  	}