From d96c2069d9453ba455540e045046ead19e885ff7 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 18 Oct 2011 11:44:39 +0200 Subject: tac: use only one temporary file, with multiple nonseekable inputs * src/tac.c (temp_stream): New function, factored out of... (copy_to_temp): ...here. (tac_nonseekable): Don't free or fclose, now that we reuse the file. Suggested by Ambrose Feinstein. * THANKS.in: Update. --- src/tac.c | 120 +++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 71 insertions(+), 49 deletions(-) (limited to 'src') diff --git a/src/tac.c b/src/tac.c index 7d99595e2..6a7e51081 100644 --- a/src/tac.c +++ b/src/tac.c @@ -418,53 +418,78 @@ record_or_unlink_tempfile (char const *fn, FILE *fp ATTRIBUTE_UNUSED) #endif -/* Copy from file descriptor INPUT_FD (corresponding to the named FILE) to - a temporary file, and set *G_TMP and *G_TEMPFILE to the resulting stream - and file name. Return true if successful. */ - +/* A wrapper around mkstemp that gives us both an open stream pointer, + FP, and the corresponding FILE_NAME. Always return the same FP/name + pair, rewinding/truncating it upon each reuse. */ static bool -copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) +temp_stream (FILE **fp, char **file_name) { - static char *template = NULL; - static char const *tempdir; - - if (template == NULL) + static char *tempfile = NULL; + static FILE *tmp_fp; + if (tempfile == NULL) { - char *t = getenv ("TMPDIR"); - tempdir = t ? t : DEFAULT_TMPDIR; - template = file_name_concat (tempdir, "tacXXXXXX", NULL); - } + char const *t = getenv ("TMPDIR"); + char const *tempdir = t ? t : DEFAULT_TMPDIR; + tempfile = file_name_concat (tempdir, "tacXXXXXX", NULL); + + /* FIXME: there's a small window between a successful mkstemp call + and the unlink that's performed by record_or_unlink_tempfile. + If we're interrupted in that interval, this code fails to remove + the temporary file. On systems that define DONT_UNLINK_WHILE_OPEN, + the window is much larger -- it extends to the atexit-called + unlink_tempfile. + FIXME: clean up upon fatal signal. Don't block them, in case + $TMPFILE is a remote file system. */ + + int fd = mkstemp (tempfile); + if (fd < 0) + { + error (0, errno, _("cannot create temporary file in %s"), + quote (tempdir)); + goto Reset; + } - /* FIXME: there's a small window between a successful mkstemp call - and the unlink that's performed by record_or_unlink_tempfile. - If we're interrupted in that interval, this code fails to remove - the temporary file. On systems that define DONT_UNLINK_WHILE_OPEN, - the window is much larger -- it extends to the atexit-called - unlink_tempfile. - FIXME: clean up upon fatal signal. Don't block them, in case - $TMPFILE is a remote file system. */ - - char *tempfile = xstrdup (template); - int fd = mkstemp (tempfile); - if (fd < 0) - { - error (0, errno, _("cannot create temporary file in %s"), - quote (tempdir)); - free (tempfile); - return false; - } + tmp_fp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); + if (! tmp_fp) + { + error (0, errno, _("cannot open %s for writing"), quote (tempfile)); + close (fd); + unlink (tempfile); + Reset: + free (tempfile); + tempfile = NULL; + return false; + } - FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); - if (! tmp) + record_or_unlink_tempfile (tempfile, tmp_fp); + } + else { - error (0, errno, _("cannot open %s for writing"), quote (tempfile)); - close (fd); - unlink (tempfile); - free (tempfile); - return false; + if (fseek (tmp_fp, 0, SEEK_SET) < 0 + || ftruncate (fileno (tmp_fp), 0) < 0) + { + error (0, errno, _("failed to rewind stream for %s"), + quote (tempfile)); + return false; + } } - record_or_unlink_tempfile (tempfile, tmp); + *fp = tmp_fp; + *file_name = tempfile; + return true; +} + +/* Copy from file descriptor INPUT_FD (corresponding to the named FILE) to + a temporary file, and set *G_TMP and *G_TEMPFILE to the resulting stream + and file name. Return true if successful. */ + +static bool +copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) +{ + FILE *fp; + char *file_name; + if (!temp_stream (&fp, &file_name)) + return false; while (1) { @@ -477,26 +502,25 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) goto Fail; } - if (fwrite (G_buffer, 1, bytes_read, tmp) != bytes_read) + if (fwrite (G_buffer, 1, bytes_read, fp) != bytes_read) { - error (0, errno, _("%s: write error"), quotearg_colon (tempfile)); + error (0, errno, _("%s: write error"), quotearg_colon (file_name)); goto Fail; } } - if (fflush (tmp) != 0) + if (fflush (fp) != 0) { - error (0, errno, _("%s: write error"), quotearg_colon (tempfile)); + error (0, errno, _("%s: write error"), quotearg_colon (file_name)); goto Fail; } - *g_tmp = tmp; - *g_tempfile = tempfile; + *g_tmp = fp; + *g_tempfile = file_name; return true; Fail: - fclose (tmp); - free (tempfile); + fclose (fp); return false; } @@ -512,8 +536,6 @@ tac_nonseekable (int input_fd, const char *file) return false; bool ok = tac_seekable (fileno (tmp_stream), tmp_file); - fclose (tmp_stream); - free (tmp_file); return ok; } -- cgit v1.2.3-54-g00ecf