[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-gnulib] invalid use of errno after ferror
|
From: |
Paul Eggert |
|
Subject: |
Re: [Bug-gnulib] invalid use of errno after ferror |
|
Date: |
16 Sep 2003 14:24:37 -0700 |
|
User-agent: |
Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 |
Here's a revised patch to fix invalid use of errno after ferror in
coreutils.
As noted below for csplit.c, od.c, uniq.c, this patch still isn't
perfect, but fixing it will take more time than I have right now, and
anyway I think this fix improves on the current code which often
reports incorrect errno values in those situations.
2003-09-16 Paul Eggert <address@hidden>
Don't assume ferror sets errno. Bug reported by Bruno Haible.
* lib/getndelim2.c, lib/linebuffer.c, lib/readutmp.c: Sync with
gnulib.
* src/comm.c (compare_files): Save errno after input error,
to report proper errno value.
* src/fold.c (fold_file): Likewise.
* src/od.c (check_and_close, skip, read_char, read_block): Likewise.
* src/paste.c (paste_serial): Likewise.
* src/unexpand.c (unexpand): Likewise.
* src/csplit.c (close_output_file): Don't report bogus errno value
after ferror discovers an output error. We don't know the proper
errno value, since it might have been caused by any of a whole
bunch of calls, and it might have been trashed in the meantime.
Fixing this problem will require much more extensive changes;
in the meantime just say "write error".
* src/od.c (check_and_close, dump, dump_strings): Likewise.
* src/uniq.c (check_file): Likewise.
* src/join.c (get_line): Report error right away if I/O fails,
so that the proper errno value is used.
* src/tac.c (tac_seekable, tac_file, save_stdin): Likewise.
* src/tee.c (tee): Likewise.
* src/uniq.c (check_file): Likewise.
* src/od.c (skip): If a read fails, don't retry it later, so
that we report the proper errno.
* src/tac.c (tac_mem): Don't return a value; nobody uses it.
* src/tee.c (tee): Once a write failure has occurred, don't bother
writing anything more to that stream.
* src/uniq.c (check_file): Check for ferror (stdout) even if
ostream == stdout.
* src/yes.c (UNROLL): Remove.
(main): Exit immediately when write failure is detected.
Simplify code by assigning to argv when argc == 1.
Index: lib/getndelim2.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/lib/getndelim2.c,v
retrieving revision 1.2
diff -p -u -r1.2 getndelim2.c
--- lib/getndelim2.c 10 Sep 2003 08:42:04 -0000 1.2
+++ lib/getndelim2.c 16 Sep 2003 20:05:12 -0000
@@ -70,7 +70,7 @@ getndelim2 (char **lineptr, size_t *line
{
/* Here always *lineptr + *linesize == read_pos + nbytes_avail. */
- register int c = getc (stream);
+ register int c;
/* We always want at least one char left in the buffer, since we
always (unless we get an error while reading the first char)
@@ -95,7 +95,8 @@ getndelim2 (char **lineptr, size_t *line
}
}
- if (c == EOF || ferror (stream))
+ c = getc (stream);
+ if (c == EOF)
{
/* Return partial line, if any. */
if (read_pos == *lineptr)
Index: lib/linebuffer.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/lib/linebuffer.c,v
retrieving revision 1.14
diff -p -u -r1.14 linebuffer.c
--- lib/linebuffer.c 10 Sep 2003 08:50:00 -0000 1.14
+++ lib/linebuffer.c 16 Sep 2003 20:05:12 -0000
@@ -45,7 +45,9 @@ initbuffer (struct linebuffer *linebuffe
that ends in a non-newline character. Do not null terminate.
Therefore the stream can contain NUL bytes, and the length
(including the newline) is returned in linebuffer->length.
- Return NULL upon error, or when STREAM is empty.
+ Return NULL when stream is empty. Return NULL and set errno upon
+ error; callers can distinguish this case from the empty case by
+ invoking ferror (stream).
Otherwise, return LINEBUFFER. */
struct linebuffer *
readlinebuffer (struct linebuffer *linebuffer, FILE *stream)
@@ -55,7 +57,7 @@ readlinebuffer (struct linebuffer *lineb
char *p = linebuffer->buffer;
char *end = buffer + linebuffer->size; /* Sentinel. */
- if (feof (stream) || ferror (stream))
+ if (feof (stream))
return NULL;
do
@@ -63,7 +65,7 @@ readlinebuffer (struct linebuffer *lineb
c = getc (stream);
if (c == EOF)
{
- if (p == buffer)
+ if (p == buffer || ferror (stream))
return NULL;
if (p[-1] == '\n')
break;
Index: lib/readutmp.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/lib/readutmp.c,v
retrieving revision 1.16
diff -p -u -r1.16 readutmp.c
--- lib/readutmp.c 11 Apr 2003 12:21:59 -0000 1.16
+++ lib/readutmp.c 16 Sep 2003 20:05:13 -0000
@@ -104,23 +104,33 @@ read_utmp (const char *filename, int *n_
if (utmp == NULL)
return 1;
- fstat (fileno (utmp), &file_stats);
+ if (fstat (fileno (utmp), &file_stats) != 0)
+ {
+ int e = errno;
+ fclose (utmp);
+ errno = e;
+ return 1;
+ }
size = file_stats.st_size;
- if (size > 0)
- buf = xmalloc (size);
- else
+ buf = xmalloc (size);
+ n_read = fread (buf, sizeof *buf, size / sizeof *buf, utmp);
+ if (ferror (utmp))
{
+ int e = errno;
+ free (buf);
fclose (utmp);
+ errno = e;
+ return 1;
+ }
+ if (fclose (utmp) != 0)
+ {
+ int e = errno;
+ free (buf);
+ errno = e;
return 1;
}
- /* Use < instead of != in case the utmp just grew. */
- n_read = fread (buf, 1, size, utmp);
- if (ferror (utmp) || fclose (utmp) == EOF
- || n_read < size)
- return 1;
-
- *n_entries = size / sizeof (STRUCT_UTMP);
+ *n_entries = n_read;
*utmp_buf = buf;
return 0;
Index: src/comm.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/comm.c,v
retrieving revision 1.67
diff -p -u -r1.67 comm.c
--- src/comm.c 23 Jul 2003 07:29:54 -0000 1.67
+++ src/comm.c 16 Sep 2003 20:10:30 -0000
@@ -144,6 +144,9 @@ compare_files (char **infiles)
/* streams[i] holds the input stream for file i. */
FILE *streams[2];
+ /* errno values for each stream. */
+ int saved_errno[2];
+
int i, ret = 0;
/* Initialize the storage. */
@@ -159,6 +162,7 @@ compare_files (char **infiles)
}
thisline[i] = readlinebuffer (thisline[i], streams[i]);
+ saved_errno[i] = errno;
}
while (thisline[0] || thisline[1])
@@ -198,16 +202,27 @@ compare_files (char **infiles)
/* Step the file the line came from.
If the files match, step both files. */
if (order >= 0)
- thisline[1] = readlinebuffer (thisline[1], streams[1]);
+ {
+ thisline[1] = readlinebuffer (thisline[1], streams[1]);
+ saved_errno[1] = errno;
+ }
if (order <= 0)
- thisline[0] = readlinebuffer (thisline[0], streams[0]);
+ {
+ thisline[0] = readlinebuffer (thisline[0], streams[0]);
+ saved_errno[0] = errno;
+ }
}
/* Free all storage and close all input streams. */
for (i = 0; i < 2; i++)
{
free (lb1[i].buffer);
- if (ferror (streams[i]) || fclose (streams[i]) == EOF)
+ if (ferror (streams[i]))
+ {
+ error (0, saved_errno[i], "%s", infiles[i]);
+ ret = 1;
+ }
+ if (fclose (streams[i]) != 0)
{
error (0, errno, "%s", infiles[i]);
ret = 1;
Index: src/csplit.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/csplit.c,v
retrieving revision 1.115
diff -p -u -r1.115 csplit.c
--- src/csplit.c 23 Jul 2003 07:29:54 -0000 1.115
+++ src/csplit.c 16 Sep 2003 20:10:31 -0000
@@ -986,9 +986,15 @@ close_output_file (void)
{
if (output_stream)
{
- if (ferror (output_stream) || fclose (output_stream) == EOF)
+ if (ferror (output_stream))
{
- error (0, errno, _("write error for `%s'"), output_filename);
+ error (0, 0, _("write error for `%s'"), output_filename);
+ output_stream = NULL;
+ cleanup_fatal ();
+ }
+ if (fclose (output_stream) != 0)
+ {
+ error (0, errno, "%s", output_filename);
output_stream = NULL;
cleanup_fatal ();
}
Index: src/fold.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/fold.c,v
retrieving revision 1.71
diff -p -u -r1.71 fold.c
--- src/fold.c 11 Aug 2003 14:35:52 -0000 1.71
+++ src/fold.c 16 Sep 2003 20:10:32 -0000
@@ -126,6 +126,7 @@ fold_file (char *filename, int width)
int offset_out = 0; /* Index in `line_out' for next char. */
static char *line_out = NULL;
static int allocated_out = 0;
+ int saved_errno;
if (STREQ (filename, "-"))
{
@@ -209,12 +210,14 @@ fold_file (char *filename, int width)
line_out[offset_out++] = c;
}
+ saved_errno = errno;
+
if (offset_out)
fwrite (line_out, sizeof (char), (size_t) offset_out, stdout);
if (ferror (istream))
{
- error (0, errno, "%s", filename);
+ error (0, saved_errno, "%s", filename);
if (!STREQ (filename, "-"))
fclose (istream);
return 1;
Index: src/join.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/join.c,v
retrieving revision 1.113
diff -p -u -r1.113 join.c
--- src/join.c 23 Jul 2003 07:29:54 -0000 1.113
+++ src/join.c 16 Sep 2003 20:10:32 -0000
@@ -261,6 +261,8 @@ get_line (FILE *fp, struct line *line)
if (! readlinebuffer (&line->buf, fp))
{
+ if (ferror (fp))
+ error (EXIT_FAILURE, errno, _("read error"));
free (line->buf.buffer);
line->buf.buffer = NULL;
return 0;
Index: src/od.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/od.c,v
retrieving revision 1.137
diff -p -u -r1.137 od.c
--- src/od.c 23 Jul 2003 07:29:54 -0000 1.137
+++ src/od.c 16 Sep 2003 20:10:35 -0000
@@ -982,10 +982,11 @@ open_next_file (void)
it is not standard input. Return nonzero if there has been an error
on in_stream or stdout; return zero otherwise. This function will
report more than one error only if both a read and a write error
- have occurred. */
+ have occurred. IN_ERRNO, if nonzero, is the error number
+ corresponding to the most recent action for IN_STREAM. */
static int
-check_and_close (void)
+check_and_close (int in_errno)
{
int err = 0;
@@ -993,7 +994,7 @@ check_and_close (void)
{
if (ferror (in_stream))
{
- error (0, errno, "%s", input_filename);
+ error (0, in_errno, _("%s: read error"), input_filename);
if (in_stream != stdin)
fclose (in_stream);
err = 1;
@@ -1009,7 +1010,7 @@ check_and_close (void)
if (ferror (stdout))
{
- error (0, errno, _("standard output"));
+ error (0, 0, _("write error"));
err = 1;
}
@@ -1062,6 +1063,7 @@ static int
skip (uintmax_t n_skip)
{
int err = 0;
+ int in_errno = 0;
if (n_skip == 0)
return 0;
@@ -1096,7 +1098,7 @@ skip (uintmax_t n_skip)
{
if (fseeko (in_stream, n_skip, SEEK_CUR) != 0)
{
- error (0, errno, "%s", input_filename);
+ in_errno = errno;
err = 1;
}
n_skip = 0;
@@ -1118,7 +1120,12 @@ skip (uintmax_t n_skip)
n_bytes_read = fread (buf, 1, n_bytes_to_read, in_stream);
n_skip -= n_bytes_read;
if (n_bytes_read != n_bytes_to_read)
- break;
+ {
+ in_errno = errno;
+ err = 1;
+ n_skip = 0;
+ break;
+ }
}
}
@@ -1132,7 +1139,7 @@ skip (uintmax_t n_skip)
err = 1;
}
- err |= check_and_close ();
+ err |= check_and_close (in_errno);
err |= open_next_file ();
}
@@ -1290,7 +1297,7 @@ read_char (int *c)
if (*c != EOF)
break;
- err |= check_and_close ();
+ err |= check_and_close (errno);
err |= open_next_file ();
}
@@ -1337,7 +1344,7 @@ read_block (size_t n, char *block, size_
if (n_read == n_needed)
break;
- err |= check_and_close ();
+ err |= check_and_close (errno);
err |= open_next_file ();
}
@@ -1482,7 +1489,7 @@ dump (void)
format_address (current_offset, '\n');
if (limit_bytes_to_format && current_offset >= end_offset)
- err |= check_and_close ();
+ err |= check_and_close (0);
return err;
}
@@ -1602,7 +1609,7 @@ dump_strings (void)
free (buf);
- err |= check_and_close ();
+ err |= check_and_close (0);
return err;
}
Index: src/paste.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/paste.c,v
retrieving revision 1.62
diff -p -u -r1.62 paste.c
--- src/paste.c 27 Aug 2003 11:41:49 -0000 1.62
+++ src/paste.c 16 Sep 2003 20:10:36 -0000
@@ -325,6 +325,7 @@ paste_serial (int nfiles, char **fnamptr
register int charnew, charold; /* Current and previous char read. */
register char *delimptr; /* Current delimiter char. */
register FILE *fileptr; /* Open for reading current file. */
+ int saved_errno;
for (; nfiles; nfiles--, fnamptr++)
{
@@ -347,6 +348,7 @@ paste_serial (int nfiles, char **fnamptr
delimptr = delims; /* Set up for delimiter string. */
charold = getc (fileptr);
+ errno = saved_errno;
if (charold != EOF)
{
/* `charold' is set up. Hit it!
@@ -381,7 +383,7 @@ paste_serial (int nfiles, char **fnamptr
if (ferror (fileptr))
{
- error (0, errno, "%s", *fnamptr);
+ error (0, saved_errno, "%s", *fnamptr);
errors = 1;
}
if (fileptr == stdin)
Index: src/tac.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/tac.c,v
retrieving revision 1.102
diff -p -u -r1.102 tac.c
--- src/tac.c 23 Jul 2003 07:29:55 -0000 1.102
+++ src/tac.c 16 Sep 2003 20:10:36 -0000
@@ -314,7 +314,8 @@ tac_seekable (int input_fd, const char *
read_size = file_pos;
file_pos = 0;
}
- lseek (input_fd, file_pos, SEEK_SET);
+ if (lseek (input_fd, file_pos, SEEK_SET) < 0)
+ error (0, errno, _("%s: seek failed"), file);
/* Shift the pending record data right to make room for the new.
The source and destination regions probably overlap. */
@@ -376,7 +377,7 @@ tac_file (const char *file)
}
SET_BINARY (fileno (in));
errors = tac_seekable (fileno (in), file);
- if (ferror (in) || fclose (in) == EOF)
+ if (fclose (in) != 0)
{
error (0, errno, "%s", file);
return 1;
@@ -452,10 +453,10 @@ save_stdin (FILE **g_tmp, char **g_tempf
error (EXIT_FAILURE, errno, _("stdin: read error"));
if (fwrite (G_buffer, 1, bytes_read, tmp) != bytes_read)
- break;
+ error (EXIT_FAILURE, errno, "%s", tempfile);
}
- if (ferror (tmp) || fflush (tmp) == EOF)
+ if (fflush (tmp) != 0)
error (EXIT_FAILURE, errno, "%s", tempfile);
SET_BINARY (fileno (tmp));
@@ -515,14 +516,14 @@ memrchr (const char *buf_start, const ch
/* FIXME: describe */
-static int
+static void
tac_mem (const char *buf, size_t n_bytes, FILE *out)
{
const char *nl;
const char *bol;
if (n_bytes == 0)
- return 0;
+ return;
nl = memrchr (buf, buf + n_bytes, '\n');
bol = (nl == NULL ? buf : nl + 1);
@@ -555,7 +556,6 @@ tac_mem (const char *buf, size_t n_bytes
fwrite (buf, 1, bol - buf, out);
/* FIXME: this is work in progress.... */
- return ferror (out);
}
/* FIXME: describe */
Index: src/tee.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/tee.c,v
retrieving revision 1.63
diff -p -u -r1.63 tee.c
--- src/tee.c 23 Jul 2003 07:29:55 -0000 1.63
+++ src/tee.c 16 Sep 2003 20:10:37 -0000
@@ -209,10 +209,13 @@ tee (int nfiles, const char **files)
/* Write to all NFILES + 1 descriptors.
Standard output is the first one. */
for (i = 0; i <= nfiles; i++)
- {
- if (descriptors[i] != NULL)
- fwrite (buffer, bytes_read, 1, descriptors[i]);
- }
+ if (descriptors[i]
+ && fwrite (buffer, bytes_read, 1, descriptors[i]) != bytes_read)
+ {
+ error (0, errno, "%s", files[i]);
+ descriptors[i] = NULL;
+ ret = 1;
+ }
}
if (bytes_read == -1)
@@ -223,8 +226,7 @@ tee (int nfiles, const char **files)
/* Close the files, but not standard output. */
for (i = 1; i <= nfiles; i++)
- if (descriptors[i] != NULL
- && (ferror (descriptors[i]) || fclose (descriptors[i]) == EOF))
+ if (descriptors[i] && fclose (descriptors[i]) != 0)
{
error (0, errno, "%s", files[i]);
ret = 1;
Index: src/unexpand.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/unexpand.c,v
retrieving revision 1.75
diff -p -u -r1.75 unexpand.c
--- src/unexpand.c 23 Jul 2003 07:29:55 -0000 1.75
+++ src/unexpand.c 16 Sep 2003 20:10:37 -0000
@@ -235,6 +235,7 @@ unexpand (void)
int next_tab_column; /* Column the next tab stop is on. */
int convert = 1; /* If nonzero, perform translations. */
unsigned int pending = 0; /* Pending columns of blanks. */
+ int saved_errno;
fp = next_file ((FILE *) NULL);
if (fp == NULL)
@@ -246,6 +247,7 @@ unexpand (void)
for (;;)
{
c = getc (fp);
+ saved_errno = errno;
if (c == ' ' && convert && column < TAB_STOP_SENTINEL)
{
@@ -325,6 +327,7 @@ unexpand (void)
if (c == EOF)
{
+ errno = saved_errno;
fp = next_file (fp);
if (fp == NULL)
break; /* No more files. */
Index: src/uniq.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/uniq.c,v
retrieving revision 1.104
diff -p -u -r1.104 uniq.c
--- src/uniq.c 23 Jul 2003 07:29:55 -0000 1.104
+++ src/uniq.c 16 Sep 2003 20:10:38 -0000
@@ -335,7 +335,11 @@ check_file (const char *infile, const ch
char *thisfield;
size_t thislen;
if (readlinebuffer (thisline, istream) == 0)
- break;
+ {
+ if (ferror (istream))
+ goto closefiles;
+ break;
+ }
thisfield = find_field (thisline);
thislen = thisline->length - 1 - (thisfield - thisline->buffer);
match = !different (thisfield, prevfield, thislen, prevlen);
@@ -377,9 +381,11 @@ check_file (const char *infile, const ch
if (ferror (istream) || fclose (istream) == EOF)
error (EXIT_FAILURE, errno, _("error reading %s"), infile);
+ if (ferror (ostream))
+ error (EXIT_FAILURE, 0, _("error writing %s"), outfile);
/* Close ostream only if it's not stdout -- the latter is closed
via the atexit-invoked close_stdout. */
- if (ostream != stdout && (ferror (ostream) || fclose (ostream) == EOF))
+ if (ostream != stdout && fclose (ostream) != 0)
error (EXIT_FAILURE, errno, _("error writing %s"), outfile);
free (lb1.buffer);
Index: src/yes.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/yes.c,v
retrieving revision 1.46
diff -p -u -r1.46 yes.c
--- src/yes.c 23 Jul 2003 07:29:55 -0000 1.46
+++ src/yes.c 16 Sep 2003 20:10:38 -0000
@@ -32,9 +32,6 @@
#define AUTHORS "David MacKenzie"
-/* How many iterations between ferror checks. */
-#define UNROLL 10000
-
/* The name this program was run with. */
char *program_name;
@@ -81,34 +78,19 @@ main (int argc, char **argv)
if (argc == 1)
{
- while (1)
- {
- int i;
- for (i = 0; i < UNROLL; i++)
- puts ("y");
- if (ferror (stdout))
- break;
- }
+ argv[1] = "y";
+ argc = 2;
}
- else
+
+ for (;;)
{
- while (1)
- {
- int i;
- for (i = 0; i < UNROLL; i++)
- {
- int j;
- for (j = 1; j < argc; j++)
- {
- fputs (argv[j], stdout);
- putchar (j == argc - 1 ? '\n' : ' ');
- }
- }
- if (ferror (stdout))
- break;
- }
+ int i;
+ for (i = 1; i < argc; i++)
+ if (fputs (argv[i], stdout) == EOF
+ || putchar (i == argc - 1 ? '\n' : ' ') == EOF)
+ {
+ error (0, errno, _("standard output"));
+ exit (EXIT_FAILURE);
+ }
}
-
- error (0, errno, _("standard output"));
- exit (EXIT_FAILURE);
}