Re: [PATCH] split: fix cases where -n l/... creates extraneous files

[Jim Meyering]

Pádraig Brady wrote:
> Subject: [PATCH] split: fix cases where -n l/... creates extraneous files
>
> * src/split.c (lines_chunk_split): Ensure that data is only
> written to stdout when k specified.  Also ensure that
> extra files are not created when there is more data available
> than reported in the file size.
> * tests/misc/split-lchunk: Verify that split -n l/k/n doesn't
> generate any files, and that -n l/n always generates n files.
> * NEWS: Mention the fix.
> ---
>  NEWS                    |    3 +++
>  src/split.c             |   17 ++++++++++++++---
>  tests/misc/split-lchunk |   16 ++++++++++++++++
>  3 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a71f832..502a5c6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,9 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>    printf '%d' '"' no longer accesses out-of-bounds memory in the diagnostic.
>    [bug introduced in sh-utils-1.16]
>
> +  split --number l/... no longer creates extraneous files in certain cases.
> +  [bug introduced in coreutils-8.8]
> +
...
> diff --git a/tests/misc/split-lchunk b/tests/misc/split-lchunk
> index 0f1693b..762138b 100755
> --- a/tests/misc/split-lchunk
> +++ b/tests/misc/split-lchunk
> @@ -34,6 +34,12 @@ split -n l/10 /dev/null || fail=1
>  test "$(stat -c %s x* | uniq -c | sed 's/^ *//; s/ /x/')" = "10x0" || fail=1
>  rm x??
>
> +# Ensure the correct number of files written
> +# even if there is more data than the reported file size
> +split -n l/2 /dev/zero
> +test "$(stat -c %s x* | wc -l)" = '2' || fail=1
> +rm x??
> +
>  # Ensure --elide-empty-files is honored
>  split -e -n l/10 /dev/null || fail=1
>  stat x?? 2>/dev/null && fail=1
> @@ -66,9 +72,19 @@ for ELIDE_EMPTY in '' '-e'; do
>      test "$DEBUGGING" && printf "\n---io-blk-size=$IO_BLKSIZE $ELIDE_EMPTY\n"
>      for N in 6 8 12 15 22; do
>        rm -f x*
> +
> +      if test -z "$ELIDE_EMPTY"; then
> +        split ---io-blksize=$IO_BLKSIZE $ELIDE_EMPTY -n l/2/$N in > chunk.k
> +        stat x* >/dev/null 2>/dev/null && fail=1
> +      fi
> +
>        split ---io-blksize=$IO_BLKSIZE $ELIDE_EMPTY -n l/$N in
>        echo $(stat -c "%02s" x*) >> out
>
> +      if test -z "$ELIDE_EMPTY"; then
> +        compare chunk.k xab || fail=1
> +      fi
> +

Nice work.
The only nit I see is that there's no reason to redirect stdout here,
since it should be unused:

diff --git a/tests/misc/split-lchunk b/tests/misc/split-lchunk
index 762138b..12d76e8 100755
--- a/tests/misc/split-lchunk
+++ b/tests/misc/split-lchunk
@@ -75,7 +75,7 @@ for ELIDE_EMPTY in '' '-e'; do

       if test -z "$ELIDE_EMPTY"; then
         split ---io-blksize=$IO_BLKSIZE $ELIDE_EMPTY -n l/2/$N in > chunk.k
-        stat x* >/dev/null 2>/dev/null && fail=1
+        stat x* 2>/dev/null && fail=1
       fi

       split ---io-blksize=$IO_BLKSIZE $ELIDE_EMPTY -n l/$N in