On Sat, Dec 19, 2020 at 5:00 PM Nipunn Koorapati via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> The logic added to check for negative pathspec match by c0192df630
> (refspec: add support for negative refspecs, 2020-09-30) looks at
> refspec->src assuming it is never NULL, however when
> remote.origin.push is set to ":", then refspec->src is NULL,
> causing a segfault within strcmp
>
> Added testing for this case in fetch-negative-refspec
A couple minor comments below...
> Signed-off-by: Nipunn Koorapati <nipunn@xxxxxxxxxxx>
> ---
> diff --git a/remote.c b/remote.c
> @@ -751,9 +751,13 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
> + } else if (refspec->matching) {
> + /* For the special matching refspec, any query should match */
> + string_list_append(&reversed, needle);
> + } else if (refspec->src == NULL) {
> + BUG("refspec->src should not be null here");
I realize that you copied Junio's example, but style on this project
is to write this as:
} else if (!refspec->src) {
...
> diff --git a/t/t5582-fetch-negative-refspec.sh b/t/t5582-fetch-negative-refspec.sh
> @@ -186,4 +186,19 @@ test_expect_success "fetch --prune with negative refspec" '
> +test_expect_success "push with matching ':' refspec" '
> + (
> + cd two &&
> + git config remote.one.push : &&
> + # Fails w/ tip behind counterpart - but should not segfault
> + test_must_fail git push one master &&
> +
> + git config remote.one.push +: &&
> + # Fails w/ tip behind counterpart - but should not segfault
> + test_must_fail git push one master &&
> +
> + git config --unset remote.one.push
> + )
> +'
If anything in this test fails prior to the final `git config
--unset`, then that cleanup command won't be executed, which might
negatively impact tests which follow. To ensure cleanup whether the
test succeeds or fails, use test_config(). Unfortunately,
test_config() has the limitation that it can't be used in subshells,
so you may have to restructure the test a bit, perhaps like this:
test_config remote.one.push : &&
(
cd two &&
test_must_fail git push one master &&
git config remote.one.push +: &&
test_must_fail git push one master
)
Driving the test with a for-loop and taking advantage of -C to avoid
the subshell is also an option:
for v in : +:
do
test_config -C two remote.one.push $v &&
test_must_fail git -C two push one master || return 1
done
