Package: coreutils;
Reported by: Ari Pollak <aripollak <at> gmail.com>
Date: Wed, 27 Apr 2011 06:46:02 UTC
Severity: normal
Done: Jim Meyering <jim <at> meyering.net>
Bug is archived. No further changes may be made.
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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.