Re: [PATCH 5/5] refspec.c: use rhs in parse_refspec instead of potentially uninitialized item->dst

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

Stefan Beller <sbeller@google.com> writes:

> 'item->dst' has not been assigned if '!rhs' is true. As the caller is allowed to pass in uninitialized
> memory (we don't assume 'item' was zeroed out before calling), this fixes an access to
> uninitialized memory.

Did I miss the other 4 patches that this might depend on it?

> diff --git a/refspec.c b/refspec.c
> index c59a4ccf1e5..ea169dec0d3 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -108,7 +108,7 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
>  		 * - empty is not allowed.
>  		 * - otherwise it must be a valid looking ref.
>  		 */
> -		if (!item->dst) {
> +		if (!rhs) {
>  			if (check_refname_format(item->src, flags))
>  				return 0;
>  		} else if (!*item->dst) {

Perhaps a better fisx is to explicitly assign NULL to item->dst when
we see there is no right-hand-side.

Aside from the "uninitialized" issue, the original if/else cascade
around here makes a lot more sense than the updated version.  If we
do not leave item->dst uninitialized, the control (and the readers'
understanding) can flow without having to carry the invariant
"item->dst is set ONLY when rhs != NULL" throughout this codepath,
in order to understand that this if/else cascade is asking: is
pointer NULL?  then do one thing, otherwise is pointee NUL? then do
another thing, otherwise we have a non-empty string so do something
on it.