[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#11631: Head command does not position file pointer correctly for neg
From: |
Jim Meyering |
Subject: |
bug#11631: Head command does not position file pointer correctly for negative line count |
Date: |
Tue, 05 Jun 2012 20:12:43 +0200 |
Jim Meyering wrote:
>>> Here's the start of a proper patch.
>>> To come: mention this in NEWS and add a test.
Here's the complete patch.
Figuring out how to trigger the bug in my patch
(s/start_pos/pos/) that Pádraig spotted was interesting.
No prior test case exercised any of the related code,
at least not with the BUFSIZ value of 8KiB that I have here.
As for when the bug was introduced, I confirmed that it's present
at least as far back as textutils-2.1 with this:
printf '\nok\n' > k; (head -n-1>/dev/null; grep o || echo FAIL) < k
and there was no significant seek-related change prior to that.
>From 295ee521bc1a4f473ee8b7b5a4be32c5b5c7386f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 5 Jun 2012 12:24:49 +0200
Subject: [PATCH] head: with --lines=-N (-n-N) reset file pointer on seekable
input
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/head.c (elide_tail_lines_seekable): Reset file pointer
after printing up to an end-relative line-counted offset.
Anoop Sharma reported the problem and suggested the fix.
* tests/misc/head-pos: Add coverage via a very similar, existing test.
Also add coverage for a previously untested block of code.
* tests/misc/head-elide-tail ($READ_BUFSIZE): Update to 8192, to
match the value of BUFSIZ I see today on Fedora 17/x86_64 (unrelated
to this fix).
* NEWS (Bug fixes): Mention it.
Improved-by: Pádraig Brady
---
NEWS | 7 +++++++
src/head.c | 8 ++++++++
tests/misc/head-elide-tail | 2 +-
tests/misc/head-pos | 26 +++++++++++++++++++-------
4 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/NEWS b/NEWS
index 7c7c2c3..c0e5fdb 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,13 @@ GNU coreutils NEWS -*-
outline -*-
processes will not intersperse their output.
[the bug dates back to the initial implementation]
+ head --lines=-N (-n-N) now resets the read pointer of a seekable input file.
+ This means that "head -n-3" no longer consumes all of its input, and lines
+ not output by head may be processed by other programs. For example, this
+ command now prints the final line, 2, while before it would print nothing:
+ seq 2 > k; (head -n-1 > /dev/null; cat) < k
+ [This bug was present in "the beginning".]
+
ls --color would mis-color relative-named symlinks in /
[bug introduced in coreutils-8.17]
diff --git a/src/head.c b/src/head.c
index d7e83b7..0d5e1b2 100644
--- a/src/head.c
+++ b/src/head.c
@@ -667,6 +667,14 @@ elide_tail_lines_seekable (const char *pretty_filename,
int fd,
Don't bother testing for failure for such a small amount.
Any failure will be detected upon close. */
fwrite (buffer, 1, n + 1, stdout);
+
+ /* Set file pointer to the byte after what we've output. */
+ if (lseek (fd, pos + n + 1, SEEK_SET) < 0)
+ {
+ error (0, errno, "%s: failed to reset file pointer",
+ quote (pretty_filename));
+ return false;
+ }
return true;
}
}
diff --git a/tests/misc/head-elide-tail b/tests/misc/head-elide-tail
index de4896b..85f509d 100755
--- a/tests/misc/head-elide-tail
+++ b/tests/misc/head-elide-tail
@@ -26,7 +26,7 @@ $ENV{PROG} = 'head';
@ENV{qw(LANGUAGE LANG LC_ALL)} = ('C') x 3;
# This should match the definition in head.c.
-my $READ_BUFSIZE = 4096;
+my $READ_BUFSIZE = 8192;
my @Tests =
(
diff --git a/tests/misc/head-pos b/tests/misc/head-pos
index 3d96261..fa3284e 100755
--- a/tests/misc/head-pos
+++ b/tests/misc/head-pos
@@ -21,12 +21,24 @@
print_ver_ head
(echo a; echo b) > in || framework_failure_
-
-(head -n 1 >/dev/null; cat) < in > out || fail=1
-cat <<EOF > exp
-b
-EOF
-
-compare exp out || fail=1
+echo b > exp || framework_failure_
+
+for i in -1 1; do
+ (head -n $i >/dev/null; cat) < in > out || fail=1
+ compare exp out || fail=1
+done
+
+# Exercise the (start_pos < pos) block in elide_tail_lines_seekable.
+# So far, this is the only test to do that.
+# Do that by creating a file larger than BUFSIZ (I've seen 128K) and
+# elide a suffix of it (by line count) that is also larger than BUFSIZ.
+# 50000 lines times 6 bytes per line gives us enough leeway even on a
+# system with a BUFSIZ of 256K.
+n_lines=50000
+seq 70000 > in2 || framework_failure_
+echo $n_lines > exp-n || framework_failure_
+
+(head -n-$n_lines>/dev/null; wc -l) < in2 > n
+compare exp-n n || fail=1
Exit $fail
--
1.7.11.rc1
Message not available
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Anoop Sharma, 2012/06/06
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Jim Meyering, 2012/06/06
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Eric Blake, 2012/06/06
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Jim Meyering, 2012/06/06
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Anoop Sharma, 2012/06/07
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Jim Meyering, 2012/06/07
- bug#11631: closed (Re: bug#11631: Head command does not position file pointer correctly for negative line count), Anoop Sharma, 2012/06/08