Re: [coreutils] [PATCH] Fix fadvice hint in fold

[Jim Meyering]

Andreas Schwab wrote:
> * src/fold.c (fold_file): Advice istream, not stdin.
> ---
>  src/fold.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/fold.c b/src/fold.c
> index d585856..4da834b 100644
> --- a/src/fold.c
> +++ b/src/fold.c
> @@ -143,7 +143,7 @@ fold_file (char const *filename, size_t width)
>        return false;
>      }
>
> -  fadvise (stdin, FADVISE_SEQUENTIAL);
> +  fadvise (istream, FADVISE_SEQUENTIAL);

Thanks for the fix!
I'm curious: how did you find the bug?

This bug appears to cause little harm: it merely inhibits
the optimization, and only when not reading from stdin.

For example, I tried to see if there was any harm
(or even a failed syscall) when using the buggy version of fold
on a closed standard input stream:

    $ echo foo > k
    $ strace -e fadvise64 ./fold k 0>&-
    fadvise64(0, 0, 0, POSIX_FADV_SEQUENTIAL) = 0
    foo

But fold worked, and surprisingly, fadvise64 did not fail.
I've adjusted the commit log and will push this soon:

>From 9c589e694f3c235d723ae80533f3bf09a5ac3702 Mon Sep 17 00:00:00 2001
From: Andreas Schwab <address@hidden>
Date: Sat, 16 Oct 2010 13:32:04 +0200
Subject: [PATCH] fold: fix fadvise hint

* src/fold.c (fold_file): Apply fadvise to istream, not stdin.
This bug would have inhibited the fadvise optimization when not
reading from standard input.
---
 src/fold.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/fold.c b/src/fold.c
index d585856..4da834b 100644
--- a/src/fold.c
+++ b/src/fold.c
@@ -143,7 +143,7 @@ fold_file (char const *filename, size_t width)
       return false;
     }

-  fadvise (stdin, FADVISE_SEQUENTIAL);
+  fadvise (istream, FADVISE_SEQUENTIAL);

   while ((c = getc (istream)) != EOF)
     {
-- 
1.7.3.1.526.g2ee4