From: Pádraig Brady Date: Tue, 18 Jul 2023 14:39:05 +0000 (+0100) Subject: tac: fall back to /tmp if $TMPDIR is unavailable X-Git-Tag: v9.4~70 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1b86b70dd5118462ba9388742ebd8459d0b84156;p=thirdparty%2Fcoreutils.git tac: fall back to /tmp if $TMPDIR is unavailable This also refactors temp_stream() to its own module, in preparation for use by split. * src/tac.c: Refactor temp_stream() out to ... * src/temp-stream.c: ... A new module mostly refactored from tac, but uses tmpdir to more robustly support $TMPDIR, while falling back to /tmp if not available. * src/temp-stream.h: The new module interface. * src/local.mk: Reference the new module from tac. * tests/tac/tac.pl: Adjust to non failing missing $TMPDIR. * po/POTFILES.in: Reference the new module with translatable strings. * NEWS: Mention the user visible improvements to tac TMPDIR handling. --- diff --git a/NEWS b/NEWS index b21a2b74d8..2708371c6b 100644 --- a/NEWS +++ b/NEWS @@ -68,6 +68,8 @@ GNU coreutils NEWS -*- outline -*- split now uses more tuned access patterns for its potentially large input. This was seen to improve throughput by 5% when reading from SSD. + tac now falls back to '/tmp' if a configured $TMPDIR is unavailable. + * Noteworthy changes in release 9.3 (2023-04-18) [stable] diff --git a/po/POTFILES.in b/po/POTFILES.in index 9dbbe32f3a..695bf6fa83 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -125,6 +125,7 @@ src/tac-pipe.c src/tac.c src/tail.c src/tee.c +src/temp-stream.c src/test.c src/timeout.c src/touch.c diff --git a/src/local.mk b/src/local.mk index 3e3d3f6966..dd50ba7aef 100644 --- a/src/local.mk +++ b/src/local.mk @@ -59,6 +59,7 @@ noinst_HEADERS = \ src/set-fields.h \ src/statx.h \ src/system.h \ + src/temp-stream.h \ src/uname.h EXTRA_DIST += \ @@ -395,6 +396,8 @@ src_arch_SOURCES = src/uname.c src/uname-arch.c src_cut_SOURCES = src/cut.c src/set-fields.c src_numfmt_SOURCES = src/numfmt.c src/set-fields.c +src_tac_SOURCES = src/tac.c src/temp-stream.c + src_tail_SOURCES = src/tail.c src/iopoll.c src_tee_SOURCES = src/tee.c src/iopoll.c diff --git a/src/tac.c b/src/tac.c index e52d4b7f26..285f99a746 100644 --- a/src/tac.c +++ b/src/tac.c @@ -45,7 +45,7 @@ tac -r -s '.\| #include "filenamecat.h" #include "safe-read.h" -#include "stdlib--.h" +#include "temp-stream.h" #include "xbinary-io.h" /* The official name of this program (e.g., no 'g' prefix). */ @@ -55,18 +55,6 @@ tac -r -s '.\| proper_name ("Jay Lepreau"), \ proper_name ("David MacKenzie") -#if defined __MSDOS__ || defined _WIN32 -/* Define this to non-zero on systems for which the regular mechanism - (of unlinking an open file and expecting to be able to write, seek - back to the beginning, then reread it) doesn't work. E.g., on Windows - and DOS systems. */ -# define DONT_UNLINK_WHILE_OPEN 1 -#endif - - -#ifndef DEFAULT_TMPDIR -# define DEFAULT_TMPDIR "/tmp" -#endif /* The number of bytes per atomic read. */ #define INITIAL_READSIZE 8192 @@ -381,113 +369,6 @@ tac_seekable (int input_fd, char const *file, off_t file_pos) } } -#if DONT_UNLINK_WHILE_OPEN - -/* FIXME-someday: remove all of this DONT_UNLINK_WHILE_OPEN junk. - Using atexit like this is wrong, since it can fail - when called e.g. 32 or more times. - But this isn't a big deal, since the code is used only on WOE/DOS - systems, and few people invoke tac on that many nonseekable files. */ - -static char const *file_to_remove; -static FILE *fp_to_close; - -static void -unlink_tempfile (void) -{ - fclose (fp_to_close); - unlink (file_to_remove); -} - -static void -record_or_unlink_tempfile (char const *fn, FILE *fp) -{ - if (!file_to_remove) - { - file_to_remove = fn; - fp_to_close = fp; - atexit (unlink_tempfile); - } -} - -#else - -static void -record_or_unlink_tempfile (char const *fn, MAYBE_UNUSED FILE *fp) -{ - unlink (fn); -} - -#endif - -/* 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 -temp_stream (FILE **fp, char **file_name) -{ - static char *tempfile = nullptr; - static FILE *tmp_fp; - if (tempfile == nullptr) - { - char const *t = getenv ("TMPDIR"); - char const *tempdir = t ? t : DEFAULT_TMPDIR; - tempfile = mfile_name_concat (tempdir, "tacXXXXXX", nullptr); - if (tempdir == nullptr) - { - error (0, 0, _("memory exhausted")); - return false; - } - - /* 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, _("failed to create temporary file in %s"), - quoteaf (tempdir)); - goto Reset; - } - - tmp_fp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); - if (! tmp_fp) - { - error (0, errno, _("failed to open %s for writing"), - quoteaf (tempfile)); - close (fd); - unlink (tempfile); - Reset: - free (tempfile); - tempfile = nullptr; - return false; - } - - record_or_unlink_tempfile (tempfile, tmp_fp); - } - else - { - clearerr (tmp_fp); - if (fseeko (tmp_fp, 0, SEEK_SET) < 0 - || ftruncate (fileno (tmp_fp), 0) < 0) - { - error (0, errno, _("failed to rewind stream for %s"), - quoteaf (tempfile)); - return false; - } - } - - *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 the number of bytes copied, or -1 on error. */ diff --git a/src/temp-stream.c b/src/temp-stream.c new file mode 100644 index 0000000000..9150304b32 --- /dev/null +++ b/src/temp-stream.c @@ -0,0 +1,165 @@ +/* temp-stream.c -- provide a stream to a per process temp file + + Copyright (C) 2023 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +#include +#include + +#include "stdlib--.h" /* For mkstemp that returns safer FDs. */ +#include "system.h" +#include "tmpdir.h" + +#include "temp-stream.h" + + +#if defined __MSDOS__ || defined _WIN32 +/* Define this to non-zero on systems for which the regular mechanism + (of unlinking an open file and expecting to be able to write, seek + back to the beginning, then reread it) doesn't work. E.g., on Windows + and DOS systems. */ +# define DONT_UNLINK_WHILE_OPEN 1 +#endif + +#if DONT_UNLINK_WHILE_OPEN + +/* FIXME-someday: remove all of this DONT_UNLINK_WHILE_OPEN junk. + Using atexit like this is wrong, since it can fail + when called e.g. 32 or more times. + But this isn't a big deal, since the code is used only on WOE/DOS + systems, and few people invoke tac on that many nonseekable files. */ + +static char const *file_to_remove; +static FILE *fp_to_close; + +static void +unlink_tempfile (void) +{ + fclose (fp_to_close); + unlink (file_to_remove); +} + +static void +record_or_unlink_tempfile (char const *fn, FILE *fp) +{ + if (!file_to_remove) + { + file_to_remove = fn; + fp_to_close = fp; + atexit (unlink_tempfile); + } +} + +#else + +static void +record_or_unlink_tempfile (char const *fn, MAYBE_UNUSED FILE *fp) +{ + unlink (fn); +} + +#endif + +/* 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. + + Note this honors $TMPDIR, unlike the standard defined tmpfile(). + + Returns TRUE on success. */ +bool +temp_stream (FILE **fp, char **file_name) +{ + static char *tempfile = nullptr; + static FILE *tmp_fp; + if (tempfile == nullptr) + { + char *tempbuf = nullptr; + size_t tempbuf_len = 128; + + while (true) + { + if (! (tempbuf = realloc (tempbuf, tempbuf_len))) + { + error (0, errno, _("failed to make temporary file name")); + return false; + } + + if (path_search (tempbuf, tempbuf_len, nullptr, "cutmp", true) == 0) + break; + + if (errno != EINVAL || PATH_MAX / 2 < tempbuf_len) + { + error (0, errno == EINVAL ? ENAMETOOLONG : errno, + _("failed to make temporary file name")); + return false; + } + + tempbuf_len *= 2; + } + + tempfile = tempbuf; + + /* 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 + $TMPDIR is a remote file system. */ + + int fd = mkstemp (tempfile); + if (fd < 0) + { + error (0, errno, _("failed to create temporary file %s"), + quoteaf (tempfile)); + goto Reset; + } + + tmp_fp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); + if (! tmp_fp) + { + error (0, errno, _("failed to open %s for writing"), + quoteaf (tempfile)); + close (fd); + unlink (tempfile); + Reset: + free (tempfile); + tempfile = nullptr; + return false; + } + + record_or_unlink_tempfile (tempfile, tmp_fp); + } + else + { + clearerr (tmp_fp); + if (fseeko (tmp_fp, 0, SEEK_SET) < 0 + || ftruncate (fileno (tmp_fp), 0) < 0) + { + error (0, errno, _("failed to rewind stream for %s"), + quoteaf (tempfile)); + return false; + } + } + + *fp = tmp_fp; + if (file_name) + *file_name = tempfile; + return true; +} diff --git a/src/temp-stream.h b/src/temp-stream.h new file mode 100644 index 0000000000..6c32e0c8d5 --- /dev/null +++ b/src/temp-stream.h @@ -0,0 +1,6 @@ +/* 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. + + Note this honors $TMPDIR, unlike the standard defined tmpfile(). */ +extern bool temp_stream (FILE **fp, char **file_name); diff --git a/tests/tac/tac.pl b/tests/tac/tac.pl index 1c188e41c2..dd67c80eec 100755 --- a/tests/tac/tac.pl +++ b/tests/tac/tac.pl @@ -74,9 +74,7 @@ my @Tests = ['pipe-bad-tmpdir', {ENV => "TMPDIR=$bad_dir"}, {IN_PIPE => "a\n"}, - {ERR_SUBST => "s,'$bad_dir': .*,...,"}, - {ERR => "$prog: failed to create temporary file in ...\n"}, - {EXIT => 1}], + {OUT=>"a\n"}], # coreutils-8.5's tac would double-free its primary buffer. ['double-free', {IN=>$long_line}, {OUT=>$long_line}],