[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] grep: check stdin like other files
From: |
Jim Meyering |
Subject: |
Re: [PATCH] grep: check stdin like other files |
Date: |
Sun, 01 Jan 2012 14:50:16 +0100 |
Paul Eggert wrote:
> I found this bug while thinking about improving the performance
> of 'grep' on binary files.
>
> =====
>
> * src/main.c (grepfile): Revamp tests for input files so that
> standard input is tested like other files. For example, report
> an error if standard input equals standard output.
> Prefer open+fstat to stat+open if possible, as open+fstat is
> usually a bit faster and avoids a race condition.
> * tests/in-eq-out-infloop: Add tests for cases like
> 'grep pat <file >>file'.
Nice one.
Please add a NEWS entry.
A fix for another disk-filling "infloop" bug deserves one.
> ---
> src/main.c | 110
> ++++++++++++++++++++++++++++-------------------
> tests/in-eq-out-infloop | 33 ++++++++------
> 2 files changed, 84 insertions(+), 59 deletions(-)
>
> diff --git a/src/main.c b/src/main.c
...
> +
> + stat_result = (desc < 0
> + ? stat (file, &stats->stat)
> + : fstat (desc, &stats->stat));
> + if (stat_result != 0)
> + {
IMHO, "_result" is an unnecessarily generic name, and decl-after-stmt
is ok for grep proper (but not dfa.c), so how about this instead?
bool stat_fail = (desc < 0
? stat (file, &stats->stat)
: fstat (desc, &stats->stat));
if (stat_fail)
{
...
> + suppressible_error (filename, errno);
> + if (file)
> + close (desc);
> + return 1;
> + }
...
> diff --git a/tests/in-eq-out-infloop b/tests/in-eq-out-infloop
...
> -compare err.exp err || fail=1
> + # Require an exit status of 2.
> + # grep-2.8 and earlier would infloop with $arg = out.
> + # grep-2.9 and earlier would infloop with $arg = - or $arg = ''.
s/2.9/2.10 since the bug is present in the latest release.
> + timeout 10 grep 0 $arg < out >> out 2> err; st=$?
> + test $st = 2 || fail=1
> + compare err.exp err || fail=1