From d5a7410d22075ecf9f2597d2516bb9914cd55049 Mon Sep 17 00:00:00 2001 From: Zoltan Klinger Date: Tue, 21 Oct 2014 16:56:03 +1100 Subject: [PATCH 1/4] grep: fix match highlighting for combined patterns with context lines When git grep is run with combined patterns such as '-e p1 --and -e p2' and surrounding context lines are requested, the output contains incorrectly highlighted matches. Consider the following output (highlighted matches are surrounded by '*' characters): $ cat testfile foo a foo b foo bar baz bar foo bar x bar y $ git grep -n -C2 -e foo --and -e bar testfile testfile-1-*foo* a testfile-2-*foo* b testfile:3:*foo* *bar* testfile:4:baz *bar* *foo* testfile-5-*bar* x testfile-6-*bar* y Lines 1, 2, 5 and 6 do not match the combined patterns, they only contain incorrectly highlighted 'false positives'. Modify the show_line() function in grep.c to highlight matches only on lines that match the combined pattern. Do not highlight matches on lines that provide only context or contain only the function name of the match. The output of the same command after the change: $ git grep -n -C2 -e foo --and -e bar testfile testfile-1-foo a testfile-2-foo b testfile:3:*foo* *bar* testfile:4:baz *bar* *foo* testfile-5-bar x testfile-6-bar y Also avoid unnecessary regmatch on lines we do not split by moving the code to paint a single line piece-by-piece only to apply to the matched line. Signed-off-by: Zoltan Klinger Signed-off-by: Junio C Hamano --- grep.c | 42 ++++++++++++----------- t/t7810-grep.sh | 90 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 20 deletions(-) diff --git a/grep.c b/grep.c index c668034739258d..b363a9407bc8be 100644 --- a/grep.c +++ b/grep.c @@ -1112,31 +1112,33 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol, output_sep(opt, sign); } if (opt->color) { - regmatch_t match; - enum grep_context ctx = GREP_CONTEXT_BODY; - int ch = *eol; - int eflags = 0; + if (sign == ':') { + /* paint the hits on matched lines */ + regmatch_t match; + enum grep_context ctx = GREP_CONTEXT_BODY; + int ch = *eol; + int eflags = 0; - if (sign == ':') line_color = opt->color_selected; - else if (sign == '-') + *eol = '\0'; + while (next_match(opt, bol, eol, ctx, &match, eflags)) { + if (match.rm_so == match.rm_eo) + break; + + output_color(opt, bol, match.rm_so, line_color); + output_color(opt, bol + match.rm_so, + match.rm_eo - match.rm_so, + opt->color_match); + bol += match.rm_eo; + rest -= match.rm_eo; + eflags = REG_NOTBOL; + } + *eol = ch; + } else if (sign == '-') { line_color = opt->color_context; - else if (sign == '=') + } else if (sign == '=') { line_color = opt->color_function; - *eol = '\0'; - while (next_match(opt, bol, eol, ctx, &match, eflags)) { - if (match.rm_so == match.rm_eo) - break; - - output_color(opt, bol, match.rm_so, line_color); - output_color(opt, bol + match.rm_so, - match.rm_eo - match.rm_so, - opt->color_match); - bol += match.rm_eo; - rest -= match.rm_eo; - eflags = REG_NOTBOL; } - *eol = ch; } output_color(opt, bol, rest, line_color); opt->output(opt, "\n", 1); diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index f698001c996ea2..87d1242548b262 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -1195,4 +1195,94 @@ test_expect_success LIBPCRE 'grep -P "^ "' ' test_cmp expected actual ' +cat >expected <with leading space1 +space: line with leading space2 +space: line with leading space3 +space:line without leading space2 +EOF + +test_expect_success 'grep --color -e A -e B with context' ' + test_config color.grep.context normal && + test_config color.grep.filename normal && + test_config color.grep.function normal && + test_config color.grep.linenumber normal && + test_config color.grep.match red && + test_config color.grep.selected normal && + test_config color.grep.separator normal && + + git grep --color=always -C2 -e "with " -e space2 space | + test_decode_color >actual && + test_cmp expected actual +' + +cat >expected <with leading space2 +space- line with leading space3 +space-line without leading space2 +EOF + +test_expect_success 'grep --color -e A --and -e B with context' ' + test_config color.grep.context normal && + test_config color.grep.filename normal && + test_config color.grep.function normal && + test_config color.grep.linenumber normal && + test_config color.grep.match red && + test_config color.grep.selected normal && + test_config color.grep.separator normal && + + git grep --color=always -C2 -e "with " --and -e space2 space | + test_decode_color >actual && + test_cmp expected actual +' + +cat >expected <with leading space1 +space- line with leading space2 +space: line with leading space3 +space-line without leading space2 +EOF + +test_expect_success 'grep --color -e A --and --not -e B with context' ' + test_config color.grep.context normal && + test_config color.grep.filename normal && + test_config color.grep.function normal && + test_config color.grep.linenumber normal && + test_config color.grep.match red && + test_config color.grep.selected normal && + test_config color.grep.separator normal && + + git grep --color=always -C2 -e "with " --and --not -e space2 space | + test_decode_color >actual && + test_cmp expected actual +' + +cat >expected < +hello.c=int main(int argc, const char **argv) +hello.c-{ +hello.c: printf("Hello world.\n"); +hello.c- return 0; +hello.c- /* char ?? */ +hello.c-} +EOF + +test_expect_success 'grep --color -e A --and -e B -p with context' ' + test_config color.grep.context normal && + test_config color.grep.filename normal && + test_config color.grep.function normal && + test_config color.grep.linenumber normal && + test_config color.grep.match red && + test_config color.grep.selected normal && + test_config color.grep.separator normal && + + git grep --color=always -p -C3 -e int --and -e Hello --no-index hello.c | + test_decode_color >actual && + test_cmp expected actual +' + test_done From 2e50bf575fbaaf3b0410ae9af0c1ab6af691aa9c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 27 Oct 2014 12:54:05 -0700 Subject: [PATCH 2/4] Revert "grep: fix match highlighting for combined patterns with context lines" This reverts commit d5a7410d22075ecf9f2597d2516bb9914cd55049. It turns out that showing partial matches on the lines that are not matching ones is a feature, not a bug, modelling after ms/mc specifiers in GNU grep's environment variables GREP_COLORS. --- grep.c | 42 +++++++++++------------ t/t7810-grep.sh | 90 ------------------------------------------------- 2 files changed, 20 insertions(+), 112 deletions(-) diff --git a/grep.c b/grep.c index b363a9407bc8be..c668034739258d 100644 --- a/grep.c +++ b/grep.c @@ -1112,33 +1112,31 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol, output_sep(opt, sign); } if (opt->color) { - if (sign == ':') { - /* paint the hits on matched lines */ - regmatch_t match; - enum grep_context ctx = GREP_CONTEXT_BODY; - int ch = *eol; - int eflags = 0; + regmatch_t match; + enum grep_context ctx = GREP_CONTEXT_BODY; + int ch = *eol; + int eflags = 0; + if (sign == ':') line_color = opt->color_selected; - *eol = '\0'; - while (next_match(opt, bol, eol, ctx, &match, eflags)) { - if (match.rm_so == match.rm_eo) - break; - - output_color(opt, bol, match.rm_so, line_color); - output_color(opt, bol + match.rm_so, - match.rm_eo - match.rm_so, - opt->color_match); - bol += match.rm_eo; - rest -= match.rm_eo; - eflags = REG_NOTBOL; - } - *eol = ch; - } else if (sign == '-') { + else if (sign == '-') line_color = opt->color_context; - } else if (sign == '=') { + else if (sign == '=') line_color = opt->color_function; + *eol = '\0'; + while (next_match(opt, bol, eol, ctx, &match, eflags)) { + if (match.rm_so == match.rm_eo) + break; + + output_color(opt, bol, match.rm_so, line_color); + output_color(opt, bol + match.rm_so, + match.rm_eo - match.rm_so, + opt->color_match); + bol += match.rm_eo; + rest -= match.rm_eo; + eflags = REG_NOTBOL; } + *eol = ch; } output_color(opt, bol, rest, line_color); opt->output(opt, "\n", 1); diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 87d1242548b262..f698001c996ea2 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -1195,94 +1195,4 @@ test_expect_success LIBPCRE 'grep -P "^ "' ' test_cmp expected actual ' -cat >expected <with leading space1 -space: line with leading space2 -space: line with leading space3 -space:line without leading space2 -EOF - -test_expect_success 'grep --color -e A -e B with context' ' - test_config color.grep.context normal && - test_config color.grep.filename normal && - test_config color.grep.function normal && - test_config color.grep.linenumber normal && - test_config color.grep.match red && - test_config color.grep.selected normal && - test_config color.grep.separator normal && - - git grep --color=always -C2 -e "with " -e space2 space | - test_decode_color >actual && - test_cmp expected actual -' - -cat >expected <with leading space2 -space- line with leading space3 -space-line without leading space2 -EOF - -test_expect_success 'grep --color -e A --and -e B with context' ' - test_config color.grep.context normal && - test_config color.grep.filename normal && - test_config color.grep.function normal && - test_config color.grep.linenumber normal && - test_config color.grep.match red && - test_config color.grep.selected normal && - test_config color.grep.separator normal && - - git grep --color=always -C2 -e "with " --and -e space2 space | - test_decode_color >actual && - test_cmp expected actual -' - -cat >expected <with leading space1 -space- line with leading space2 -space: line with leading space3 -space-line without leading space2 -EOF - -test_expect_success 'grep --color -e A --and --not -e B with context' ' - test_config color.grep.context normal && - test_config color.grep.filename normal && - test_config color.grep.function normal && - test_config color.grep.linenumber normal && - test_config color.grep.match red && - test_config color.grep.selected normal && - test_config color.grep.separator normal && - - git grep --color=always -C2 -e "with " --and --not -e space2 space | - test_decode_color >actual && - test_cmp expected actual -' - -cat >expected < -hello.c=int main(int argc, const char **argv) -hello.c-{ -hello.c: printf("Hello world.\n"); -hello.c- return 0; -hello.c- /* char ?? */ -hello.c-} -EOF - -test_expect_success 'grep --color -e A --and -e B -p with context' ' - test_config color.grep.context normal && - test_config color.grep.filename normal && - test_config color.grep.function normal && - test_config color.grep.linenumber normal && - test_config color.grep.match red && - test_config color.grep.selected normal && - test_config color.grep.separator normal && - - git grep --color=always -p -C3 -e int --and -e Hello --no-index hello.c | - test_decode_color >actual && - test_cmp expected actual -' - test_done From ee901beaae9237c833e636c3766ca6caa9c225a3 Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Mon, 8 Dec 2014 18:28:35 +0700 Subject: [PATCH 3/4] index-pack: terminate object buffers with NUL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have some tricky checks in fsck that rely on a side effect of require_end_of_header(), and would otherwise easily run outside non-NUL-terminated buffers. This is a bit brittle, so let's make sure that only NUL-terminated buffers are passed around to begin with. Jeff "Peff" King contributed the detailed analysis which call paths are involved and pointed out that we also have to patch the get_data() function in unpack-objects.c, which is what Johannes "Dscho" Schindelin implemented. Signed-off-by: Nguyễn Thái Ngọc Duy Analyzed-by: Jeff King Signed-off-by: Johannes Schindelin --- builtin/index-pack.c | 4 ++-- builtin/unpack-objects.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index a369f553535164..46321176719808 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -447,7 +447,7 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size, if (type == OBJ_BLOB && size > big_file_threshold) buf = fixed_buf; else - buf = xmalloc(size); + buf = xmallocz(size); memset(&stream, 0, sizeof(stream)); git_inflate_init(&stream); @@ -552,7 +552,7 @@ static void *unpack_data(struct object_entry *obj, git_zstream stream; int status; - data = xmalloc(consume ? 64*1024 : obj->size); + data = xmallocz(consume ? 64*1024 : obj->size); inbuf = xmalloc((len < 64*1024) ? len : 64*1024); memset(&stream, 0, sizeof(stream)); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 855d94b90ba019..ac6667242c562b 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -91,7 +91,7 @@ static void use(int bytes) static void *get_data(unsigned long size) { git_zstream stream; - void *buf = xmalloc(size); + void *buf = xmallocz(size); memset(&stream, 0, sizeof(stream)); From 567ad592ffbbacccbe0c59edd50b53f1f739b471 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 8 Dec 2014 07:45:34 -0600 Subject: [PATCH 4/4] fsck: verify that we get only NUL-terminated buffers Signed-off-by: Johannes Schindelin --- fsck.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fsck.c b/fsck.c index 2fffa434a5763a..9165eb3b4c31e0 100644 --- a/fsck.c +++ b/fsck.c @@ -452,6 +452,9 @@ static int fsck_tag(struct tag *tag, const char *data, int fsck_object(struct object *obj, void *data, unsigned long size, int strict, fsck_error error_func) { + if (data && ((char *)data)[size]) + die("I got a non-NUL-terminated buffer!!!"); + if (!obj) return error_func(obj, FSCK_ERROR, "no valid object to fsck");