GNU bug report logs - #8565
gzip/bzip2/output-hook support for split

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8565 in the body.
You can then email your comments to 8565 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ari Pollak <aripollak <at> gmail.com>
To: bug-coreutils <at> gnu.org
Subject: gzip/bzip2/output-hook support for split
Date: Wed, 27 Apr 2011 00:30:41 -0400

[Message part 1 (text/plain, inline)]

I've been sitting on this for a few months expecting to finish it, but
still haven't found the time, so I'm just putting it out there now so
it isn't lost entirely. It's originally from Chandrakumar Muthaiah:
http://article.gmane.org/gmane.comp.gnu.coreutils.bugs/15684
I applied it to the latest git, added some tests, and cleaned it up a bit.

[output-hook.diff (text/x-patch, attachment)]

Message #8 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Ari Pollak <aripollak <at> gmail.com>
Cc: 8565 <at> debbugs.gnu.org, sudhach <at> chandrasudha.net
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Wed, 27 Apr 2011 22:25:56 +0200

Ari Pollak wrote:
> I've been sitting on this for a few months expecting to finish it, but
> still haven't found the time, so I'm just putting it out there now so
> it isn't lost entirely. It's originally from Chandrakumar Muthaiah:
> http://article.gmane.org/gmane.comp.gnu.coreutils.bugs/15684
> I applied it to the latest git, added some tests, and cleaned it up a bit.

Thank you for working on that.
However, we still don't have a copyright assignment
from Chandrakumar Muthaiah.  Chandrakumar, have you
started the process?

    http://git.sv.gnu.org/cgit/coreutils.git/tree/HACKING#n429

Ari, since you're contributing nontrivial
changes, would you do that, too?

Both of you, please let me know when/if you start the process.

Message #11 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 8565 <at> debbugs.gnu.org, Ari Pollak <aripollak <at> gmail.com>,
	sudhach <at> chandrasudha.net, kwzh <at> gnu.org
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Fri, 29 Apr 2011 00:51:25 +0100

On 27/04/11 21:25, Jim Meyering wrote:
> Ari Pollak wrote:
>> I've been sitting on this for a few months expecting to finish it, but
>> still haven't found the time, so I'm just putting it out there now so
>> it isn't lost entirely. It's originally from Chandrakumar Muthaiah:
>> http://article.gmane.org/gmane.comp.gnu.coreutils.bugs/15684
>> I applied it to the latest git, added some tests, and cleaned it up a bit.
> 
> Thank you for working on that.
> However, we still don't have a copyright assignment
> from Chandrakumar Muthaiah.  Chandrakumar, have you
> started the process?
> 
>     http://git.sv.gnu.org/cgit/coreutils.git/tree/HACKING#n429
> 
> Ari, since you're contributing nontrivial
> changes, would you do that, too?
> 
> Both of you, please let me know when/if you start the process.

There is also Karl's variant, and he already has assigned copyright
http://lists.gnu.org/archive/html/coreutils/2011-01/msg00023.html

cheers,
Pádraig.

Message #14 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Karl Heuer <kwzh <at> gnu.org>
Cc: 8565 <at> debbugs.gnu.org, Ari Pollak <aripollak <at> gmail.com>,
	sudhach <at> chandrasudha.net,
	Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Fri, 29 Apr 2011 10:37:20 +0200

Pádraig Brady wrote:
> On 27/04/11 21:25, Jim Meyering wrote:
>> Ari Pollak wrote:
>>> I've been sitting on this for a few months expecting to finish it, but
>>> still haven't found the time, so I'm just putting it out there now so
>>> it isn't lost entirely. It's originally from Chandrakumar Muthaiah:
>>> http://article.gmane.org/gmane.comp.gnu.coreutils.bugs/15684
>>> I applied it to the latest git, added some tests, and cleaned it up a bit.
>>
>> Thank you for working on that.
>> However, we still don't have a copyright assignment
>> from Chandrakumar Muthaiah.  Chandrakumar, have you
>> started the process?
>>
>>     http://git.sv.gnu.org/cgit/coreutils.git/tree/HACKING#n429
>>
>> Ari, since you're contributing nontrivial
>> changes, would you do that, too?
>>
>> Both of you, please let me know when/if you start the process.
>
> There is also Karl's variant, and he already has assigned copyright
> http://lists.gnu.org/archive/html/coreutils/2011-01/msg00023.html

Thanks for the reminder, Pádraig.  Karl's is more general.
It's just as well: I haven't heard from Chandrakumar.

Karl, this is a good time to add the feature.
If you don't have time to handle the remaining bits (test, doc and NEWS),
let us know and we will take care of the rest.

Message #17 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: kwzh <at> gnu.org (Karl Heuer)
Cc: 8565 <at> debbugs.gnu.org, Ari Pollak <aripollak <at> gmail.com>,
	sudhach <at> chandrasudha.net,
	Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Fri, 29 Apr 2011 12:33:53 +0200

Karl Heuer wrote:
>>Karl, this is a good time to add the feature.
>>If you don't have time to handle the remaining bits (test,
>>doc and NEWS), let us know and we will take care of the rest.
>
> I'd better let you do it; I've got too many other items in my
> queue at the moment.

Ok.  For starters, I'm squashing this onto your patch.
The type change is required to get past this compilation error:

    split.c: In function 'create':
    split.c:334:42: error: passing argument 2 of 'x2nrealloc' from incompatible pointer type [-Werror]
    ../lib/xalloc.h:191:1: note: expected 'size_t *' but argument is of type 'int *'
    cc1: all warnings being treated as errors

Also, at least initially, I'll remove the short-named (-f)
version of your new --filter option, just to be on the safe side.
People can always abbreviate using "--f".


From 6b72fcbf896e082cc797b601c0f4e2533a479981 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Fri, 29 Apr 2011 12:26:50 +0200
Subject: [PATCH] split: minor adjustments: s/int/size_t/ for new globals,
 and...

rename: open_pipes_size -> open_pipes_alloc
   and  open_pipes_len  -> n_open_pipes
---
 src/split.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/split.c b/src/split.c
index c977ceb..2b9cdfc 100644
--- a/src/split.c
+++ b/src/split.c
@@ -56,8 +56,8 @@ static int filter_pid;

 /* Array of open pipes.  */
 static int *open_pipes;
-static int open_pipes_size;
-static int open_pipes_len;
+static size_t open_pipes_alloc;
+static size_t n_open_pipes;

 /* Blocked signals.  */
 static sigset_t oldblocked;
@@ -307,7 +307,7 @@ create (const char* name)
              earlier call, otherwise this process will be holding a
              write-pipe that will prevent the earlier process from
              reading an EOF on the corresponding read-pipe.  */
-          for (j = 0; j < open_pipes_len; ++j)
+          for (j = 0; j < n_open_pipes; ++j)
             if (close (open_pipes[j]) != 0)
               error (EXIT_FAILURE, errno, _("closing prior pipe"));
           if (close (fd_pair[1]))
@@ -329,10 +329,10 @@ create (const char* name)
       if (close (fd_pair[0]) != 0)
         error (EXIT_FAILURE, errno, _("closing input pipe"));
       filter_pid = child_pid;
-      if (open_pipes_len == open_pipes_size)
-        open_pipes = (int *) x2nrealloc (open_pipes, &open_pipes_size,
+      if (n_open_pipes == open_pipes_alloc)
+        open_pipes = (int *) x2nrealloc (open_pipes, &open_pipes_alloc,
                                          sizeof *open_pipes);
-      open_pipes[open_pipes_len++] = fd_pair[1];
+      open_pipes[n_open_pipes++] = fd_pair[1];
       return fd_pair[1];
     }
 }
@@ -350,9 +350,9 @@ closeout (FILE *fp, int fd, pid_t pid, char const *name)
       if (fp == NULL && close (fd) < 0)
         error (EXIT_FAILURE, errno, "%s", name);
       int j;
-      for (j = 0; j < open_pipes_len; ++j)
+      for (j = 0; j < n_open_pipes; ++j)
         if (open_pipes[j] == fd) {
-          open_pipes[j] = open_pipes[--open_pipes_len];
+          open_pipes[j] = open_pipes[--n_open_pipes];
           break;
         }
     }
--
1.7.5.452.gcf2d0

Message #20 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: kwzh <at> gnu.org (Karl Heuer)
To: Jim Meyering <jim <at> meyering.net>
Cc: 8565 <at> debbugs.gnu.org, Ari Pollak <aripollak <at> gmail.com>, sudhach <at> chandrasudha.net, Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Fri, 29 Apr 2011 05:26:14 -0400

>Karl, this is a good time to add the feature.
>If you don't have time to handle the remaining bits (test,
>doc and NEWS), let us know and we will take care of the rest.

I'd better let you do it; I've got too many other items in my
queue at the moment.

Message #23 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: kwzh <at> gnu.org (Karl Heuer)
Cc: 8565 <at> debbugs.gnu.org, Ari Pollak <aripollak <at> gmail.com>,
	sudhach <at> chandrasudha.net,
	Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Sat, 30 Apr 2011 16:29:45 +0200

Karl Heuer wrote:
>>Karl, this is a good time to add the feature.
>>If you don't have time to handle the remaining bits (test,
>>doc and NEWS), let us know and we will take care of the rest.
>
> I'd better let you do it; I've got too many other items in my
> queue at the moment.

Hi Karl,

FYI, I've nearly finished with split-filter-related changes.
I posted a slightly modified version of your patch and a few
follow-on changes:

  http://thread.gmane.org/gmane.comp.gnu.coreutils.general/1138

And one more below.

I noticed that you've added a lot of "errno != EPIPE" tests.
And for a good reason, as you explained here:

  /* When filtering, closure of one pipe must not terminate the process,
     as there may still be other streams expecting input from us.  */
  sigemptyset (&newblocked);
  if (filter_command)
    {
      struct sigaction act;
      sigaction (SIGPIPE, NULL, &act);
      if (act.sa_handler != SIG_IGN)
        sigaddset (&newblocked, SIGPIPE);
    }
  sigprocmask (SIG_BLOCK, &newblocked, &oldblocked);


However, I presume you intended that change only
when filter_command is non-NULL.  I would not want to
suppress an error if somehow I accidentally ran a
classic split command with a non-default signal handler
and I sent it a SIGPIPE signal just as it was writing to
a file I cared about.


From baf68e2dfb8ccfbfa797917e818551b12e129aef Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sat, 30 Apr 2011 16:08:04 +0200
Subject: [PATCH] split: factor out all new EPIPE tests; exempt EPIPE only
 with --filter

* src/split.c (ignorable): New function, to encapsulate EPIPE test.
Ignore failure with errno == EPIPE only when --filter=CMD is specified.
Replace all comparisons like && errno != EPIPE with
&& ! ignorable (errno).
---
 src/split.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/split.c b/src/split.c
index 66f44fc..05315e6 100644
--- a/src/split.c
+++ b/src/split.c
@@ -131,6 +131,13 @@ static struct option const longopts[] =
   {NULL, 0, NULL, 0}
 };

+/* Return true if the errno value, ERR, is ignorable.  */
+static inline bool
+ignorable (int err)
+{
+  return filter_command && err == EPIPE;
+}
+
 static void
 set_suffix_length (uintmax_t n_units, enum Split_type split_type)
 {
@@ -346,7 +353,7 @@ create (const char *name)
 static void
 closeout (FILE *fp, int fd, pid_t pid, char const *name)
 {
-  if (fp != NULL && fclose (fp) != 0 && errno != EPIPE)
+  if (fp != NULL && fclose (fp) != 0 && ! ignorable (errno))
     error (EXIT_FAILURE, errno, "%s", name);
   if (fd >= 0)
     {
@@ -412,7 +419,7 @@ cwrite (bool new_file_flag, const char *bp, size_t bytes)
       if ((output_desc = create (outfile)) < 0)
         error (EXIT_FAILURE, errno, "%s", outfile);
     }
-  if (full_write (output_desc, bp, bytes) != bytes && errno != EPIPE)
+  if (full_write (output_desc, bp, bytes) != bytes && ! ignorable (errno))
     error (EXIT_FAILURE, errno, "%s", outfile);
 }

@@ -635,7 +642,7 @@ lines_chunk_split (uintmax_t k, uintmax_t n, char *buf, size_t bufsize,
                  large chunks from an existing file, so it's more efficient
                  to write out directly.  */
               if (full_write (STDOUT_FILENO, bp, to_write) != to_write
-                  && errno != EPIPE)
+                  && ! ignorable (errno))
                 error (EXIT_FAILURE, errno, "%s", _("write error"));
             }
           else
@@ -698,7 +705,8 @@ bytes_chunk_extract (uintmax_t k, uintmax_t n, char *buf, size_t bufsize,
         error (EXIT_FAILURE, errno, "%s", infile);
       else if (n_read == 0)
         break; /* eof.  */
-      if (full_write (STDOUT_FILENO, buf, n_read) != n_read && errno != EPIPE)
+      if (full_write (STDOUT_FILENO, buf, n_read) != n_read
+          && ! ignorable (errno))
         error (EXIT_FAILURE, errno, "%s", quote ("-"));
       start += n_read;
     }
@@ -772,7 +780,7 @@ ofile_open (of_t *files, size_t i_check, size_t nfiles)
                 error (EXIT_FAILURE, errno, "%s", files[i_check].of_name);
             }

-          if (fclose (files[i_reopen].ofile) != 0 && errno != EPIPE)
+          if (fclose (files[i_reopen].ofile) != 0 && ! ignorable (errno))
             error (EXIT_FAILURE, errno, "%s", files[i_reopen].of_name);
           files[i_reopen].ofile = NULL;
           files[i_reopen].ofd = OFD_APPEND;
@@ -856,11 +864,11 @@ lines_rr (uintmax_t k, uintmax_t n, char *buf, size_t bufsize)
               if (line_no == k && unbuffered)
                 {
                   if (full_write (STDOUT_FILENO, bp, to_write) != to_write
-                      && errno != EPIPE)
+                      && ! ignorable (errno))
                     error (EXIT_FAILURE, errno, "%s", _("write error"));
                 }
               else if (line_no == k && fwrite (bp, to_write, 1, stdout) != 1
-                       && errno != EPIPE)
+                       && ! ignorable (errno))
                 {
                   clearerr (stdout); /* To silence close_stdout().  */
                   error (EXIT_FAILURE, errno, "%s", _("write error"));
@@ -877,15 +885,15 @@ lines_rr (uintmax_t k, uintmax_t n, char *buf, size_t bufsize)
                   /* Note writing to fd, rather than flushing the FILE gives
                      an 8% performance benefit, due to reduced data copying.  */
                   if (full_write (files[i_file].ofd, bp, to_write) != to_write
-                      && errno != EPIPE)
+                      && ! ignorable (errno))
                     error (EXIT_FAILURE, errno, "%s", files[i_file].of_name);
                 }
               else if (fwrite (bp, to_write, 1, files[i_file].ofile) != 1
-                       && errno != EPIPE)
+                       && ! ignorable (errno))
                 error (EXIT_FAILURE, errno, "%s", files[i_file].of_name);
               if (file_limit)
                 {
-                  if (fclose (files[i_file].ofile) != 0 && errno != EPIPE)
+                  if (fclose (files[i_file].ofile) != 0 && ! ignorable (errno))
                     error (EXIT_FAILURE, errno, "%s", files[i_file].of_name);
                   files[i_file].ofile = NULL;
                   files[i_file].ofd = OFD_APPEND;
--
1.7.5.134.g1c08b

Message #26 received at 8565 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Ari Pollak <aripollak <at> gmail.com>
Cc: 8565 <at> debbugs.gnu.org
Subject: Re: bug#8565: gzip/bzip2/output-hook support for split
Date: Tue, 31 May 2011 10:06:37 +0200

tags 8565 notabug
close 8565
thanks

Ari Pollak wrote:
> I've been sitting on this for a few months expecting to finish it, but
> still haven't found the time, so I'm just putting it out there now so
> it isn't lost entirely. It's originally from Chandrakumar Muthaiah:
> http://article.gmane.org/gmane.comp.gnu.coreutils.bugs/15684
> I applied it to the latest git, added some tests, and cleaned it up a bit.

As you may have seen, split now supports the --filter=COMMAND
option, so I'm closing this issue.  Thanks!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.