Re: [PATCH v3] [GSOC] ref-filter: fix read invalid union member bug

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

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ZheNing Hu <adlternative@gmail.com>
>
> used_atom.u is an union, and it has different members depending on
> what atom the auxiliary data the union part of the "struct
> used_atom" wants to record. At most only one of the members can be
> valid at any one time. Since the code checks u.remote_ref without
> even making sure if the atom is "push" or "push:" (which are only
> two cases that u.remote_ref.push becomes valid), but u.remote_ref
> shares the same storage for other members of the union, the check
> was reading from an invalid member, which was the bug.
>
> Modify the condition here to check whether the atom name
> equals to "push" or starts with "push:", to avoid reading the
> value of invalid member of the union.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---

Just a final sanity check.  Is this a recent breakage or was the
code introduced at cc72385f (for-each-ref: let upstream/push
optionally report the remote name, 2017-10-05) broken from the
beginning?

I am wondering if it is easy to add a test to cover the codepath
that is affected by this change.

Thanks.

> diff --git a/ref-filter.c b/ref-filter.c
> index a0adb4551d87..213d3773ada3 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1730,7 +1730,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
>  			else
>  				v->s = xstrdup("");
>  			continue;
> -		} else if (atom->u.remote_ref.push) {
> +		} else if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:")) {
>  			const char *branch_name;
>  			v->s = xstrdup("");
>  			if (!skip_prefix(ref->refname, "refs/heads/",
>
> base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a