From 6d635d3b32f74ad9f6d4100381f6ed04f4355cf0 Mon Sep 17 00:00:00 2001 From: Ed Maste Date: Sat, 20 May 2017 11:20:03 +0000 Subject: [PATCH] bsdgrep: Correct per-line line metadata printing Metadata printing with -b, -H, or -n flags suffered from a few flaws: 1) -b/offset printing was broken when used in conjunction with -o 2) With -o, bsdgrep did not print metadata for every match/line, just the first match of a line 3) There were no tests for this Address these issues by outputting this data per-match if the -o flag is specified, and prior to outputting any matches if -o but not --color, since --color alone will not generate a new line of output for every iteration over the matches. To correct -b output, fudge the line offset as we're printing matches. While here, make sure we're using grep_printline in -A context. Context printing should *never* look at the parsing context, just the line. The tests included do not pass with gnugrep in base due to it exhibiting similar quirky behavior that bsdgrep previously exhibited. Submitted by: Kyle Evans Reviewed by: cem Differential Revision: https://reviews.freebsd.org/D10580 --- contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 36 ++++++++++++++++++++- usr.bin/grep/grep.h | 1 + usr.bin/grep/queue.c | 1 + usr.bin/grep/util.c | 35 ++++++++++++++------ 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/contrib/netbsd-tests/usr.bin/grep/t_grep.sh b/contrib/netbsd-tests/usr.bin/grep/t_grep.sh index 77f04f8608d..6accab3ca72 100755 --- a/contrib/netbsd-tests/usr.bin/grep/t_grep.sh +++ b/contrib/netbsd-tests/usr.bin/grep/t_grep.sh @@ -430,7 +430,7 @@ excessive_matches_body() done atf_check -s exit:0 -x '[ $(grep -o x test.in | wc -l) -eq 4096 ]' - #atf_check -s exit:1 -x 'grep -on x test.in | grep -v "1:x"' + atf_check -s exit:1 -x 'grep -on x test.in | grep -v "1:x"' } atf_test_case fgrep_sanity @@ -510,6 +510,39 @@ wv_combo_break_body() atf_check -s exit:1 grep -v -w "x" test2 } +atf_test_case ocolor_metadata +ocolor_metadata_head() +{ + atf_set "descr" "Check for -n/-b producing per-line metadata output" +} +ocolor_metadata_body() +{ + grep_type + if [ $? -eq $GREP_TYPE_GNU_FREEBSD ]; then + atf_expect_fail "this test does not pass with GNU grep in base" + fi + + printf "xxx\nyyyy\nzzz\nfoobarbaz\n" > test1 + check_expr="^[^:]*[0-9][^:]*:[^:]+$" + + atf_check -o inline:"1:1:xx\n" grep -bon "xx$" test1 + + atf_check -o inline:"2:4:yyyy\n" grep -bn "yy" test1 + + atf_check -o inline:"2:6:yy\n" grep -bon "yy$" test1 + + # These checks ensure that grep isn't producing bogus line numbering + # in the middle of a line. + atf_check -s exit:1 -x \ + "grep -Eon 'x|y|z|f' test1 | grep -Ev '${check_expr}'" + + atf_check -s exit:1 -x \ + "grep -En 'x|y|z|f' --color=always test1 | grep -Ev '${check_expr}'" + + atf_check -s exit:1 -x \ + "grep -Eon 'x|y|z|f' --color=always test1 | grep -Ev '${check_expr}'" +} + atf_test_case grep_nomatch_flags grep_nomatch_flags_head() { @@ -628,6 +661,7 @@ atf_init_test_cases() atf_add_test_case fgrep_sanity atf_add_test_case egrep_sanity atf_add_test_case grep_sanity + atf_add_test_case ocolor_metadata atf_add_test_case grep_nomatch_flags atf_add_test_case binary_flags atf_add_test_case badcontext diff --git a/usr.bin/grep/grep.h b/usr.bin/grep/grep.h index 906318c6700..9128984c70d 100644 --- a/usr.bin/grep/grep.h +++ b/usr.bin/grep/grep.h @@ -90,6 +90,7 @@ struct file { }; struct str { + off_t boff; off_t off; size_t len; char *dat; diff --git a/usr.bin/grep/queue.c b/usr.bin/grep/queue.c index 272dedfc91d..f094f8a972a 100644 --- a/usr.bin/grep/queue.c +++ b/usr.bin/grep/queue.c @@ -65,6 +65,7 @@ enqueue(struct str *x) item->data.dat = grep_malloc(sizeof(char) * x->len); item->data.len = x->len; item->data.line_no = x->line_no; + item->data.boff = x->boff; item->data.off = x->off; memcpy(item->data.dat, x->dat, x->len); item->data.file = x->file; diff --git a/usr.bin/grep/util.c b/usr.bin/grep/util.c index 0101c06bc58..7b0a50b1b8a 100644 --- a/usr.bin/grep/util.c +++ b/usr.bin/grep/util.c @@ -61,11 +61,12 @@ static bool first_match = true; * other useful bits */ struct parsec { - regmatch_t matches[MAX_LINE_MATCHES]; /* Matches made */ - struct str ln; /* Current line */ - size_t lnstart; /* Start of line processing */ - size_t matchidx; /* Latest used match index */ - bool binary; /* Binary file? */ + regmatch_t matches[MAX_LINE_MATCHES]; /* Matches made */ + struct str ln; /* Current line */ + size_t lnstart; /* Position in line */ + size_t matchidx; /* Latest match index */ + int printed; /* Metadata printed? */ + bool binary; /* Binary file? */ }; @@ -233,8 +234,10 @@ procfile(const char *fn) strcpy(pc.ln.file, fn); pc.ln.line_no = 0; pc.ln.len = 0; + pc.ln.boff = 0; pc.ln.off = -1; pc.binary = f->binary; + pc.printed = 0; tail = 0; last_outed = 0; same_file = false; @@ -248,9 +251,11 @@ procfile(const char *fn) mcount = mlimit; for (c = 0; c == 0 || !(lflag || qflag); ) { - /* Reset match count and line start for every line processed */ + /* Reset per-line statistics */ + pc.printed = 0; pc.matchidx = 0; pc.lnstart = 0; + pc.ln.boff = 0; pc.ln.off += pc.ln.len + 1; if ((pc.ln.dat = grep_fgetln(f, &pc.ln.len)) == NULL || pc.ln.len == 0) { @@ -305,7 +310,7 @@ procfile(const char *fn) if (t != 0 && doctx) { /* Deal with any -A context */ if (tail > 0) { - printline(&pc, '-'); + grep_printline(&pc.ln, '-'); tail--; if (Bflag > 0) clearqueue(); @@ -607,7 +612,7 @@ printline_metadata(struct str *line, int sep) if (bflag) { if (printsep) putchar(sep); - printf("%lld", (long long)line->off); + printf("%lld", (long long)(line->off + line->boff)); printsep = true; } if (printsep) @@ -632,13 +637,22 @@ printline(struct parsec *pc, int sep) /* --color and -o */ if ((oflag || color) && matchidx > 0) { - printline_metadata(&pc->ln, sep); + /* Only print metadata once per line if --color */ + if (!oflag && pc->printed == 0) + printline_metadata(&pc->ln, sep); for (i = 0; i < matchidx; i++) { match = pc->matches[i]; /* Don't output zero length matches */ if (match.rm_so == match.rm_eo) continue; - if (!oflag) + /* + * Metadata is printed on a per-line basis, so every + * match gets file metadata with the -o flag. + */ + if (oflag) { + pc->ln.boff = match.rm_so; + printline_metadata(&pc->ln, sep); + } else fwrite(pc->ln.dat + a, match.rm_so - a, 1, stdout); if (color) @@ -659,4 +673,5 @@ printline(struct parsec *pc, int sep) } } else grep_printline(&pc->ln, sep); + pc->printed++; }