undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

pickaxe: assert that we must have a needle under -G or -S

Browse Source 
Assert early in diffcore_pickaxe() that we've got a needle to work
with under -G and -S.

This code is redundant to the check -G and -S get from
parse-options.c's get_arg(), which I'm adding a test for.

This check dates back to e1b161161d (diffcore-pickaxe: fix infinite
loop on zero-length needle, 2007-01-25) when "git log -S" could send
this code into an infinite loop.

It was then later refactored in 8fa4b09fb1 (pickaxe: hoist empty
needle check, 2012-10-28) into its current form, but it seemingly
wasn't noticed that in the meantime a move to the parse-options.c API
in dea007fb4c (diff: parse separate options like -S foo, 2010-08-05)
had made it redundant.

Let's retain some of the paranoia here with a BUG(), but there's no
need to be checking this in the pickaxe_match() inner loop.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Ævar Arnfjörð Bjarmason 2021-04-12 19:15:20 +02:00 committed by Junio C Hamano
parent 03c1f14acf
commit 2e197a7592
2 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
diffcore-pickaxe.c
View File

@ -132,9 +132,6 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
oidset_contains(o->objfind, &p->two->oid)); oidset_contains(o->objfind, &p->two->oid));
} }
if (!o->pickaxe[0])
return 0;
if (o->flags.allow_textconv) { if (o->flags.allow_textconv) {
textconv_one = get_textconv(o->repo, p->one); textconv_one = get_textconv(o->repo, p->one);
textconv_two = get_textconv(o->repo, p->two); textconv_two = get_textconv(o->repo, p->two);
@ -230,6 +227,9 @@ void diffcore_pickaxe(struct diff_options *o)
kwset_t kws = NULL; kwset_t kws = NULL;
pickaxe_fn fn; pickaxe_fn fn;
if (opts & ~DIFF_PICKAXE_KIND_OBJFIND &&
(!needle || !*needle))
BUG("should have needle under -G or -S");
if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) { if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
int cflags = REG_EXTENDED | REG_NEWLINE; int cflags = REG_EXTENDED | REG_NEWLINE;
if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE) if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE)

6
t/t4209-log-pickaxe.sh
View File

@ -56,6 +56,12 @@ test_expect_success setup '
' '
test_expect_success 'usage' ' test_expect_success 'usage' '
test_expect_code 129 git log -S 2>err &&
test_i18ngrep "switch.*requires a value" err &&
test_expect_code 129 git log -G 2>err &&
test_i18ngrep "switch.*requires a value" err &&
test_expect_code 128 git log -Gregex -Sstring 2>err && test_expect_code 128 git log -Gregex -Sstring 2>err &&
grep "mutually exclusive" err && grep "mutually exclusive" err &&