On 3/11/25 19:21, Jamie Landeg-Jones wrote:
Kyle Evans <kev...@freebsd.org> wrote:

On 9/29/23 15:37, Kyle Evans wrote:
On 9/29/23 13:25, Jamie Landeg-Jones wrote:
Jamie Landeg-Jones <ja...@catflap.org> wrote:

Brilliant! Thanks for the quick response and fix. It works fine for me -
I've not managed to break it again :-)

Famous last words....

"grep -v" now produces duplicate lines! e.g. :


Alright, fine, be that way. :-) Try this on top of the existing patch:

https://people.freebsd.org/~kevans/grep-color.diff


This should be spelled:

https://people.freebsd.org/~kevans/grep-color-addition.diff

Sorry

Hi Kyle. This is an old thread from 2023.
( https://lists.freebsd.org/archives/freebsd-current/2023-September/004762.html 
)


Yikes, I completely forgot about this.

I've been running with these two patches since you posted them. I notice
that they haven't been commited, and the bug reported in the thread still
exists in current, so I'm replying to the original thread, both in the hope
that this specific problem can be fixed, and then your overall fixes be
submitted to the tree.

Everything else has worked fine all this time, but today I noticed a bug
that can be triggered like this:

  | % echo boo | /usr/bin/grep ''
  | Assertion failed: (pc->matchidx > 0), function procmatch_match, file 
/usr/src/usr.bin/grep/util.c, line 223.
  | Abort (core dumped)

This is caused by the snippet:

  |        /* Print the matching line, but only if not quiet/binary */
  |         if (mc->printmatch) {
  |                 size_t last_out;
  |
  |                 if (vflag)
  |                         assert(pc->matchidx == 0);
  |                 else
  |                         assert(pc->matchidx > 0);

In this case, pc-matchidx is validly 0. However, simply  Removing
the assert from the src causes the duplicate line issue again.

Why grep for '' ? Long story, but it seems to be allowed.
Even if it isn't allowed, it shouldn't be dumping core :-)

Anything further I can do to assist, please let me know.


If it makes you feel better, I clearly didn't even smoke test the patch against our own regression tests. =(

===> Expected failures
grep_test:zgrep_recursive -> expected_failure: unimplemented zgrep wrapper script functionality: atf-check failed; see the output of the test for details [0.029s]
===> Failed tests
grep_test:matchall -> failed: atf-check failed; see the output of the test for details [0.037s] grep_test:oflag_zerolen -> failed: atf-check failed; see the output of the test for details [0.074s] grep_test:xflag_emptypat -> failed: atf-check failed; see the output of the test for details [0.044s] grep_test:xflag_emptypat_plus -> failed: atf-check failed; see the output of the test for details [0.047s] grep_test:zgrep_empty_eflag -> failed: atf-check failed; see the output of the test for details [0.047s]
===> Summary
Results read from /root/.kyua/store/results.usr_obj_usr_src_arm64.aarch64_usr.bin_grep_tests_checkdir_usr_tests_usr.bin_grep.20250312-025551-841462.db
Test cases: 56 total, 0 skipped, 1 expected failures, 0 broken, 5 failed
Start time: 2025-03-12T02:55:51.906277Z
End time:   2025-03-12T02:55:55.435279Z
Total time: 2.930s
--

Which would have revealed:

Standard output:
Executing command [ zgrep -e  test ]

Standard error:
Fail: incorrect exit status: 134, expected: 0
stdout:

stderr:
Assertion failed: (pc->matchidx > 0), function procmatch_match, file /usr/src/usr.bin/grep/util.c, line 223.
Abort trap (core dumped)
--

I'll take a little bit to understand the patch I wrote back then, add an extra test to cover the originally-reported bug, fix the patch, then get it into Phabricator ASAP. Sorry for dropping this-

Thanks,

Kyle Evans

Reply via email to