Re: [PATCH] history: do not dereference NULL when parent tree is missing

[Patrick Steinhardt <ps@pks.im>]

On Wed, Sep 02, 2026 at 08:07:36PM +0800, zkd18cjb@mail.ustc.edu.cn wrote:
> write_ondisk_index() dereferences the return value of
> repo_parse_tree_indirect() unconditionally.  If the parent commit's
> tree object is missing from the object store (corrupt repository,
> object removed by tooling, or incomplete restore), the function
> returns NULL and "git history split" crashes with a SIGSEGV
> (release build; UBSan reports a null-pointer member access at
> builtin/history.c:789).

Nit: the information in the braces does not really add a lot of signal,
I'd just drop it.

> Guard the parse result and error out gracefully, following the
> codebase convention for objects that cannot be loaded.
> 
> Signed-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn>

Nit: your From address does not match the Signed-off-by.

[snip]
> Reproduction (verified on master @ f78ce2f7b6, x86-64 Linux):
> 
>     git init r && cd r
>     git config user.email t@t && git config user.name t
>     echo a > f && git add f && git commit -qm one
>     echo b > f && git commit -qam two
>     tree=$(git rev-parse 'HEAD^^{tree}')
>     rm .git/objects/$(echo "$tree" | cut -c1-2)/$(echo "$tree" | cut -c3-)
>     GIT_EDITOR=true git history split HEAD

We could of course add a test for this, but I don't really think that
it's worth it.

> diff --git a/builtin/history.c b/builtin/history.c
> index 000155ad9c..097631f5ba 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -786,6 +786,10 @@ static int write_ondisk_index(struct repository *repo,
>  	opts.dst_index = &index;
>  
>  	tree = repo_parse_tree_indirect(repo, oid);
> +	if (!tree) {
> +		ret = error(_("unable to parse tree %s"), oid_to_hex(oid));
> +		goto out;
> +	}

Yup, the fix looks obviously good to me, thanks!

Patrick