Subject: Re: bug#16329: Acknowledgement (`head --lines=-0' prints nothing if no newline at the EOF)
From: Pádraig Brady <[email-masked]>
To: Алексей Шилин <[email-masked]>
Cc: [email-masked]
Date: Wed, 29 Jan 2014 11:55:53 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On 01/03/2014 08:40 PM, Алексей Шилин wrote:
> Looks like I was overly cautious about decrementing an unsigned...
> 
> size_t n = bytes_read;
> while (n)
>   {
>     if (all_lines)
>       n -= n ? 1 : 0;        // ...here.
>     else
> 
> As it is under `while (n)' statement, n is always true here, and thus the ternary operator, though makes no
> harm, is needless, and the whole line can be replaced with just `n--;'. Sorry for that.
> 
> The fixed version of the original patch is attached.
> 

I've updated the patch to also handle the pipe case,
and I added a test. Will push soon.

thanks,
Pádraig.
From b1387eb3e922398ab90cf6aa63cac6dea37971fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B5=D0=B9=20=D0=A8=D0=B8=D0?=
 =?UTF-8?q?=BB=D0=B8=D0=BD?= <[email-masked]>
Date: Wed, 29 Jan 2014 01:23:46 +0000
Subject: [PATCH] head: fix --lines=-0 outputting nothing if no newline at EOF

* src/head.c (elide_tail_lines_pipe): Just output all input in
this case to avoid the issue and also avoid redundant '\n' processing.
(elide_tail_lines_seekable): Likewise.
* tests/misc/head-elide-tail.pl: Add tests for no '\n' at EOF.
* NEWS: Mention the fix.
Fixes http://bugs.gnu.org/16329
---
 NEWS                          |    4 ++++
 src/head.c                    |   27 +++++++++++++++++++++------
 tests/misc/head-elide-tail.pl |    7 +++++--
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index e091d18..f86e589 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   when reading the SELinux context for a file.
   [bug introduced in coreutils-8.22]
 
+  head --lines=-0, when the input does not contain a trailing '\n',
+  now copies all input to stdout.  Previously nothing was output in this case.
+  [bug introduced with the --lines=-N feature in coreutils-5.0.1]
+
   ln -sf now replaces symbolic links whose targets can't exist.  Previously
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]
diff --git a/src/head.c b/src/head.c
index ddaa990..ef368d7 100644
--- a/src/head.c
+++ b/src/head.c
@@ -501,6 +501,13 @@ elide_tail_lines_pipe (const char *filename, int fd, uintmax_t n_elide)
       n_read = safe_read (fd, tmp->buffer, BUFSIZ);
       if (n_read == 0 || n_read == SAFE_READ_ERROR)
         break;
+
+      if (! n_elide)
+        {
+          fwrite (tmp->buffer, 1, n_read, stdout);
+          continue;
+        }
+
       tmp->nbytes = n_read;
       tmp->nlines = 0;
       tmp->next = NULL;
@@ -636,8 +643,11 @@ elide_tail_lines_seekable (const char *pretty_filename, int fd,
       return false;
     }
 
+  /* n_lines == 0 case needs special treatment. */
+  const bool all_lines = !n_lines;
+
   /* Count the incomplete line on files that don't end with a newline.  */
-  if (bytes_read && buffer[bytes_read - 1] != '\n')
+  if (n_lines && bytes_read && buffer[bytes_read - 1] != '\n')
     --n_lines;
 
   while (1)
@@ -647,11 +657,16 @@ elide_tail_lines_seekable (const char *pretty_filename, int fd,
       size_t n = bytes_read;
       while (n)
         {
-          char const *nl;
-          nl = memrchr (buffer, '\n', n);
-          if (nl == NULL)
-            break;
-          n = nl - buffer;
+          if (all_lines)
+            n -= 1;
+          else
+            {
+              char const *nl;
+              nl = memrchr (buffer, '\n', n);
+              if (nl == NULL)
+                break;
+              n = nl - buffer;
+            }
           if (n_lines-- == 0)
             {
               /* Found it.  */
diff --git a/tests/misc/head-elide-tail.pl b/tests/misc/head-elide-tail.pl
index 758e1c9..3c178d6 100755
--- a/tests/misc/head-elide-tail.pl
+++ b/tests/misc/head-elide-tail.pl
@@ -52,6 +52,8 @@ my @Tests =
    ['elide-l2', "--lines=-1", {IN=>"a"}, {OUT=>''}],
    ['elide-l3', "--lines=-1", {IN=>"a\nb"}, {OUT=>"a\n"}],
    ['elide-l4', "--lines=-1", {IN=>"a\nb\n"}, {OUT=>"a\n"}],
+   ['elide-l5', "--lines=-0", {IN=>"a\nb\n"}, {OUT=>"a\nb\n"}],
+   ['elide-l6', "--lines=-0", {IN=>"a\nb"}, {OUT=>"a\nb"}],
   );
 
 if ($ENV{RUN_EXPENSIVE_TESTS})
@@ -80,9 +82,10 @@ if ($ENV{RUN_EXPENSIVE_TESTS})
       }
 
     $s =~ s/(.)/$1\n/g;
-    for my $file_size (0..20)
+    $s .= 'u'; # test without trailing '\n'
+    for my $file_size (0..21)
       {
-        for my $n_elide (0..20)
+        for my $n_elide (0..21)
           {
             my $input = substr $s, 0, 2 * $file_size;
             my $out_len = $n_elide < $file_size ? $file_size - $n_elide : 0;
-- 
1.7.7.6