Re: [PATCH] [GSOC] ref-filter: solve bugs caused by enumeration

From: Junio C Hamano <gitster@pobox.com>
Date: May 6, 2021, 01:53 UTC
Message-ID: <xmqqfsz01vc2.fsf@gitster.g>
In-Reply-To: <pull.949.git.1620228664666.gitgitgadget@gmail.com>

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

Show 6 quoted lines

> From: ZheNing Hu <adlternative@gmail.com>
>
> Johannes Schindelin seems to have introduced a bug in
> cc72385f(for-each-ref: let upstream/push optionally
> report the remote name), it use `atom->u.remote_ref.option`
> which is a member of enumeration in the judgment statement.

Sorry but I am not sure if our readers would understand what "a member of enumeration in the judgment statement" is (I certainly do not), and even more importantly, "bugs caused by enumeration" on the title does not hint much about what problem the patch is trying to solve.

Show 5 quoted lines

> When we use other members in the enumeration `used_atom.u`,
> and it happened to fill in `remote_ref.push`, this judgment
> may still be established and produces errors. So replace the
> judgment statement with `starts_with(name, "push")` to fix
> the error.

And this paragraph does not enlighten all that much, unfortunately.

Is it that a check refers to one member of a union without making sure that member is the one in effect in the union? I am most puzzled by the mention of "enumeration" when there does not appear to be any enum involved.

Show 22 quoted lines

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-949%2Fadlternative%2Fref-filter-enum-bug-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-949/adlternative/ref-filter-enum-bug-fix-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/949
>
>  ref-filter.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index a0adb4551d87..f467f2fbbb73 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 (starts_with(name, "push")) {
>  			const char *branch_name;
>  			v->s = xstrdup("");
>  			if (!skip_prefix(ref->refname, "refs/heads/",
>
> base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a