Re: [PATCH 08/11] revision: avoid dereferencing NULL in `add_parents_only()`

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

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

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This function resolves revision suffixes like commit^@ (all parents),
> commit^! (commit minus parents), and commit^-N (exclude Nth parent). It
> calls `get_reference()` in a loop to peel through tag objects until it
> reaches a commit.
>
> The existing NULL check after `get_reference()` only handles the
> ignore_missing case, but get_reference() can return NULL through three
> distinct paths:

Nicely spotted.  It sounds like something a test can ensure does not
to regress in the future, unless I am misreading this explanation.
Could you include such a test?

Thanks.

> diff --git a/revision.c b/revision.c
> index e91d7e1f11..7f3999b551 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1903,8 +1903,13 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
>  		return 0;
>  	while (1) {
>  		it = get_reference(revs, arg, &oid, 0);
> -		if (!it && revs->ignore_missing)
> -			return 0;
> +		if (!it) {
> +			if (revs->ignore_missing)
> +				return 0;
> +			if (revs->do_not_die_on_missing_objects)
> +				return 0;
> +			return -1;
> +		}
>  		if (it->type != OBJ_TAG)
>  			break;
>  		if (!((struct tag*)it)->tagged)