bug-grep
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]