"ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> ---
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
