Re: coreutils-8.14.116-1e18d on NetBSD 5.1 - split bug

[Jim Meyering]

Bruno Haible wrote:
>>   http://meyering.net/cu/coreutils-8.14.116-1e18d.tar.xz
>
> On NetBSD 5.1/x86, I get this test failure in particular:
>
>
> FAIL: split/l-chunk
> ===================
> split: /dev/zero: No such file or directory
> stat: cannot stat `x*': No such file or directory
> rm: cannot remove `x??': No such file or directory
>
>
> The first among these messages comes from this command:
> $ ./split -n l/2 /dev/zero
> ./split: /dev/zero: No such file or directory
>
> Single-stepping it, it gets to call
> lines_chunk_split (k=0, n=2, buf=0x7f7ffda01000 "", bufsize=65536,
> file_size=2)
> and at split.c:625 the call
>   full_read (STDIN_FILENO, buf, bufsize)
> returns 65536, the same value as bufsize. The code in line 627 is buggy:
> It uses errno even when full_read returned bufsize. But that value is
> undefined.
>
> This patch fixes the bug and make the test succeed:
>
>
> 2012-01-04  Bruno Haible  <address@hidden>
>
>       split: Avoid failure due to leftover 'errno' value.
>       * src/split.c (lines_chunk_split): Fix logic.
>
> --- src/split.c.bak     2012-01-05 03:03:16.000000000 +0100
> +++ src/split.c 2012-01-05 03:25:31.000000000 +0100
> @@ -623,11 +623,11 @@
>      {
>        char *bp = buf, *eob;
>        size_t n_read = full_read (STDIN_FILENO, buf, bufsize);
> -      n_read = MIN (n_read, file_size - n_written);
>        if (n_read < bufsize && errno)
>          error (EXIT_FAILURE, errno, "%s", infile);
>        else if (n_read == 0)
>          break; /* eof.  */
> +      n_read = MIN (n_read, file_size - n_written);
>        chunk_truncated = false;
>        eob = buf + n_read;

Thanks for the analysis and patch.
There was another problem just like that in bytes_chunk_extract.

Here's a proposed patch.
At first I was going to add a test to exercise the
other failure, say with "split -n 1/2 /dev/zero || fail=1"
(and may still add that), but currently split uses stat.st_size
for a non-regular file, and that is not valid.  I don't want to
test for behavior that will soon change.  stat's .st_size
member is valid only for regular files.  We'll have to address
that separately, maybe post-release.  I expect to make applying
split to /dev/null fail like it does for pipes and terminals:

    $ :|split -n 1/2
    split: `-': cannot determine file size
    [Exit 1]


>From adf89fec89e0e8df14881e7853a77f889891fe29 Mon Sep 17 00:00:00 2001
From: Bruno Haible <address@hidden>
Date: Thu, 5 Jan 2012 09:26:32 +0100
Subject: [PATCH] split: avoid failure due to leftover 'errno' value

* src/split.c (lines_chunk_split): Fix logic bug that led to
unwarranted failure of "split -n l/2 /dev/zero" on NetBSD 5.1.
The same would happen when splitting a growing file, where
open/lseek-end gives one size, but by the time we read, there
is more data available.
(bytes_chunk_extract): Likewise.
* NEWS (Bug fixes): Mention this.
But introduced with the chunk-selecting feature in v8.7-25-gbe10739.

Co-authored-by: Jim Meyering <address@hidden>
---
 NEWS        |    6 ++++++
 src/split.c |    4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index ed6f831..ceca056 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,12 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   and NcFsd file systems.  This did not affect Unix/Linux-based kernels.
   [bug introduced in coreutils-8.0, when rm began using fts]

+  split -n 1/2 FILE no longer fails when operating on a growing file, or
+  (on some systems) when operating on a non-regular file like /dev/zero.
+  It would report "/dev/zero: No such file or directory" even though
+  the file obviously exists.  Same for -n l/2.
+  [bug introduced in coreutils-8.8, with the addition of the -n option]
+
   stat -f now recognizes the FhGFS and PipeFS file system types.

   tac no longer fails to handle two or more non-seekable inputs
diff --git a/src/split.c b/src/split.c
index 5b00fe8..2eb343b 100644
--- a/src/split.c
+++ b/src/split.c
@@ -623,11 +623,11 @@ lines_chunk_split (uintmax_t k, uintmax_t n, char *buf, 
size_t bufsize,
     {
       char *bp = buf, *eob;
       size_t n_read = full_read (STDIN_FILENO, buf, bufsize);
-      n_read = MIN (n_read, file_size - n_written);
       if (n_read < bufsize && errno)
         error (EXIT_FAILURE, errno, "%s", infile);
       else if (n_read == 0)
         break; /* eof.  */
+      n_read = MIN (n_read, file_size - n_written);
       chunk_truncated = false;
       eob = buf + n_read;

@@ -718,11 +718,11 @@ bytes_chunk_extract (uintmax_t k, uintmax_t n, char *buf, 
size_t bufsize,
   while (start < end)
     {
       size_t n_read = full_read (STDIN_FILENO, buf, bufsize);
-      n_read = MIN (n_read, end - start);
       if (n_read < bufsize && errno)
         error (EXIT_FAILURE, errno, "%s", infile);
       else if (n_read == 0)
         break; /* eof.  */
+      n_read = MIN (n_read, end - start);
       if (full_write (STDOUT_FILENO, buf, n_read) != n_read
           && ! ignorable (errno))
         error (EXIT_FAILURE, errno, "%s", quote ("-"));
--
1.7.8.2.329.g04d73