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

[Jim Meyering]

Jim Meyering wrote:

> 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.

I noticed and fixed that typo: s/But/Bug/
while adding this test:

commit 931e0f2a708965001857d60cedf1b1940389cbe6
Author: Bruno Haible <address@hidden>
Date:   Thu Jan 5 09:26:32 2012 +0100

    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.
    * tests/split/l-chunk: The latter case was not exercised.
    Add code to do that.
    Bug introduced with the chunk-selecting feature in v8.7-25-gbe10739.

    Co-authored-by: Jim Meyering <address@hidden>

diff --git a/tests/split/l-chunk b/tests/split/l-chunk
index dd92b70..c4e6968 100755
--- a/tests/split/l-chunk
+++ b/tests/split/l-chunk
@@ -40,6 +40,9 @@ split -n l/2 /dev/zero
 test "$(stat -c %s x* | wc -l)" = '2' || fail=1
 rm x??

+# Repeat the above,  but with 1/2, not l/2:
+split -n 1/2 /dev/zero || fail=1
+
 # Ensure --elide-empty-files is honored
 split -e -n l/10 /dev/null || fail=1
 stat x?? 2>/dev/null && fail=1