|
View:
New views
20 Messages
—
Rating Filter:
Alert me
|
| < Prev | 1 - 2 | Next > |
|
|
temp file suffixes: mktemp DWIMNow that glibc 2.11 has mkostemps, and I'm working on adding that to gnulib, it
would be nice to expose the idea of an explicit suffix to temporary file names. But rather than require the user to count how long their suffix is, I imagine it would make more sense to give mktemp(1) some do-what-I-mean smarts. That is, I envision: mktemp foo-XXXXXX.txt calling mk[o]stemps("foo-XXXXXX.txt",4,0), automatically figuring out that the final run of XXXXXX in the template implies a suffix of length 4, to create a file such as "foo-abcdef.txt". Maybe it's even worth an option in case a user wants an explicit X in the suffix, as in: mktemp --suffix-len=1 foo-XXXXXXX to create a file named foo-abcdefX, although that starts to be a bit of overkill. Meanwhile, if the user supplies less than 6 X, I think it would also be nice to supply the missing X rather than facing an EINVAL from too few X, as in: mktemp foo- calling mkstemp("foo-XXXXXX"). Then again, this would be a slight change from the current codebase, since 'mktemp foo-XXX' currently creates a file such as "foo-abc" rather than giving EINVAL or creating "foo-abcdef". Finally, is there any reason that we guarantee that more than six X will be munged, even though glibc munges only the last six X? In other words, since the default template is tmp.XXXXXXXXXX, do we want to continue to guarantee that we generate a file such as tmp.abcdef1234, or are we okay with generating tmp.XXXXabcdef? Or do we even shorten the default template to just six X, since that seems to be enough to guarantee success. Thoughts? -- Eric Blake |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> Now that glibc 2.11 has mkostemps, and I'm working on adding that to gnulib, it Good to hear it. > would be nice to expose the idea of an explicit suffix to temporary file > names. But rather than require the user to count how long their suffix is, I > imagine it would make more sense to give mktemp(1) some do-what-I-mean smarts. > That is, I envision: > > mktemp foo-XXXXXX.txt > > calling mk[o]stemps("foo-XXXXXX.txt",4,0), automatically figuring out that the > final run of XXXXXX in the template implies a suffix of length 4, to create a > file such as "foo-abcdef.txt". > > Maybe it's even worth an option in case a user wants an explicit X in the > suffix, as in: > > mktemp --suffix-len=1 foo-XXXXXXX Here's another way to do it: mktemp --suffix=X foo-XXXXXX > to create a file named foo-abcdefX, although that starts to be a bit of > overkill. Meanwhile, if the user supplies less than 6 X, I think it would also > be nice to supply the missing X rather than facing an EINVAL from too few X, as > in: > > mktemp foo- > > calling mkstemp("foo-XXXXXX"). I disagree. IMHO, "mktemp foo-" is an error, and requires a diagnostic and nonzero exit. > Then again, this would be a slight change from > the current codebase, since 'mktemp foo-XXX' currently creates a file such > as "foo-abc" rather than giving EINVAL or creating "foo-abcdef". > > Finally, is there any reason that we guarantee that more than six X will be > munged, even though glibc munges only the last six X? In other words, since GNU mktemp does that because the original mktemp did that. Who knows, someone may actually require more than 6 random bytes. I for one appreciate not having to have 6 all the time. > the default template is tmp.XXXXXXXXXX, do we want to continue to guarantee > that we generate a file such as tmp.abcdef1234, Why change? > or are we okay with generating > tmp.XXXXabcdef? Or do we even shorten the default template to just six X, > since that seems to be enough to guarantee success. > > Thoughts? |
|
|
Re: temp file suffixes: mktemp DWIMJim Meyering <jim <at> meyering.net> writes:
> > the default template is tmp.XXXXXXXXXX, do we want to continue to guarantee > > that we generate a file such as tmp.abcdef1234, > > Why change? Good point. As part of my gnulib work, I guess that means I'll have to regenerate the coreutils patch to gen_tempname that lets us continue replacing an arbitrary number of X (as long as it is at least 3), rather than the gnulib/glibc approach of exactly 6. > > I imagine it would make more sense to give mktemp(1) some do-what-I-mean smarts. > > That is, I envision: > > > > mktemp foo-XXXXXX.txt > > > > calling mk[o]stemps("foo-XXXXXX.txt",4,0), automatically figuring out that the > > final run of XXXXXX in the template implies a suffix of length 4, to create a > > file such as "foo-abcdef.txt". > > > > Maybe it's even worth an option in case a user wants an explicit X in the > > suffix, as in: > > > > mktemp --suffix-len=1 foo-XXXXXXX > > Here's another way to do it: > > mktemp --suffix=X foo-XXXXXX Nice idea. Putting it all together, how about the following semantics: mktemp a => error, no run of X mktemp aXX => error, run of X is too short mktemp XXX => generates 3-character name (if possible) mktemp aXXX.b => generates 6-character name (if possible) mktemp --suffix=.b aXXX => longer spelling of the above line mktemp aXXX --suffix=.b => likewise, if not POSIXLY_CORRECT mktemp aXXXbXXX => generates aXXXb123 (only the last run changed) mktemp --suffix=X aXXX => only way to generate a123X instead of a1234 mktemp --suffix=.txt file => error, no run of X mktemp --suffix=.txt aXXXb => error, explicit --suffix requires trailing X The changes from the existing behavior: Addition of a --suffix option (does it warrant a short option? Probably not until it has had more field testing). Explicit use of this argument requires trailing X in the template. Addition of DWIM magic if --suffix is not present. Currently, mktemp aXXX.b errors out, stating that there are too few X's in the template. The new behavior recognizes any run of X, not just at the tail, and this goes a long way to meet user expectations: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316 -- Eric Blake |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
... >> Here's another way to do it: >> >> mktemp --suffix=X foo-XXXXXX > > Nice idea. Putting it all together, how about the following semantics: > > mktemp a => error, no run of X > mktemp aXX => error, run of X is too short > mktemp XXX => generates 3-character name (if possible) > mktemp aXXX.b => generates 6-character name (if possible) > mktemp --suffix=.b aXXX => longer spelling of the above line > mktemp aXXX --suffix=.b => likewise, if not POSIXLY_CORRECT > mktemp aXXXbXXX => generates aXXXb123 (only the last run changed) > mktemp --suffix=X aXXX => only way to generate a123X instead of a1234 > mktemp --suffix=.txt file => error, no run of X > mktemp --suffix=.txt aXXXb => error, explicit --suffix requires trailing X Nice, comprehensive set of examples. > The changes from the existing behavior: > > Addition of a --suffix option (does it warrant a short option? Probably not > until it has had more field testing). Explicit use of this argument requires > trailing X in the template. I agree. > Addition of DWIM magic if --suffix is not present. Currently, mktemp aXXX.b > errors out, stating that there are too few X's in the template. The new > behavior recognizes any run of X, not just at the tail, and this goes a long > way to meet user expectations: > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316 All sounds good. Thanks! |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake <ebb9 <at> byu.net> writes:
> Now that glibc 2.11 has mkostemps, and I'm working on adding that to gnulib, it > would be nice to expose the idea of an explicit suffix to temporary file > names. But rather than require the user to count how long their suffix is, I > imagine it would make more sense to give mktemp(1) some do-what-I-mean smarts. Before I do that, how about documenting what mktemp currently does. The proposed patch documents the current behavior, although it probably needs some tweaks (and corresponding tweaks in mktemp.c) based this email. mktemp --help is not very obvious that --tmpdir is implied if no TEMPLATE is given. Right now, 'mktemp --help' claims that --tmpdir and -p are distinct, but the code says they are almost the same (they both set use_dest_dir=true and dest_dir_arg=optarg). The only difference is that --tmpdir has an optional argument, while -p has a mandatory argument, so dest_dir_arg can be NULL if you use --tmpdir but not if you use -p. By the way, the optional argument to --subdir has some interesting effects, because we are not consistent on whether we check optarg for being the empty string: mkdir --subdir="$dir" => if $dir is empty, try $TMPDIR; if that is empty, use /tmp mkdir --subdir="$dir" -t => if $TMPDIR is empty, use $dir; if that is empty, use . In other words, the only thing that I can see -t doing is whether we first check $TMPDIR or the argument to -p/--tmpdir, and restricts us from using subdirectories in the template (is this restriction really necessary)? It doesn't even print a deprecation warning or have a FIXME date with proposed removal. So, what if we just undeprecate -t (by adding a long option), and document/implement things as follows: -p DIR, --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is empty or not supplied, use $TMPDIR if set, else /tmp -t, --favor-tmpdir with -p, favor $TMPDIR over the DIR argument; otherwise use $TMPDIR or /tmp instead of current directory TEMPLATE may contain slashes, but intermediate directories must exist. If -p or -t is specified, TEMPLATE must not be absolute. Also, is there a cleanup hole? For most code paths, an exit status of 1 means that the temporary file could not be created. But consider: mktemp >/dev/full This currently gives exit status 1, because the atexit() handler recognizes failure to print the file name to stdout, but leaves the temporary file around. Should we go ahead and manually flush/close stdout, rather than relying on close_stdout, so that we can then remove the just-created file if we detect write failure? That way, if we fail to inform the user what just got created, we are at least avoiding littering their file system with a random file. Fortunately, 'mktemp >&-' does not make the mistake of printing the just- created file as the contents of that file, since we close the fd returned by mkstemp before printing anything. At any rate, here's the proposed documentation patch: From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 10:47:42 -0700 Subject: [PATCH] doc: document mktemp * doc/coreutils.texi (mktemp invocation): New node. --- doc/coreutils.texi | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 142 insertions(+), 2 deletions(-) diff --git a/doc/coreutils.texi b/doc/coreutils.texi index ec5bcfb..1fd1bec 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -31,7 +31,6 @@ @c FIXME: the following need documentation @c * [: (coreutils)[ invocation. File/string tests. @c * pinky: (coreutils)pinky invocation. FIXME. -@c * mktemp: (coreutils)mktemp invocation. FIXME. @dircategory Individual utilities @direntry @@ -80,6 +79,7 @@ * mkdir: (coreutils)mkdir invocation. Create directories. * mkfifo: (coreutils)mkfifo invocation. Create FIFOs (named pipes). * mknod: (coreutils)mknod invocation. Create special files. +* mktemp: (coreutils)mktemp invocation. Create temporary files. * mv: (coreutils)mv invocation. Rename files. * nice: (coreutils)nice invocation. Modify niceness. * nl: (coreutils)nl invocation. Number lines and write files. @@ -193,7 +193,7 @@ Top * Printing text:: echo printf yes * Conditions:: false true test expr * Redirection:: tee -* File name manipulation:: dirname basename pathchk +* File name manipulation:: dirname basename pathchk mktemp * Working context:: pwd stty printenv tty * User information:: id logname whoami groups users who * System context:: date arch uname hostname hostid uptime @@ -378,6 +378,7 @@ Top * basename invocation:: Strip directory and suffix from a file name * dirname invocation:: Strip non-directory suffix from a file name * pathchk invocation:: Check file name validity and portability +* mktemp invocation:: Create temporary file or directory Working context @@ -11876,6 +11877,7 @@ File name manipulation * basename invocation:: Strip directory and suffix from a file name. * dirname invocation:: Strip non-directory suffix from a file name. * pathchk invocation:: Check file name validity and portability. +* mktemp invocation:: Create temporary file or directory. @end menu @@ -12050,6 +12052,144 @@ pathchk invocation 1 otherwise. @end display +@node mktemp invocation +@section @command{mktemp}: Create temporary file or directory + +@pindex mktemp +@cindex file names, creating temporary +@cindex directory, creating temporary +@cindex temporary files and directories + +@command{mktemp} manages the creation of temporary files and +directories. Synopsis: + +@example +mktemp [@var{option}]@dots{} [@var{template}] +@end example + +Safely create a temporary file or directory based on @var{template}, +and print its name. If given, @var{template} must end in at least +three consecutive @samp{X}. If omitted, the template +@samp{tmp.XXXXXXXXXX} is used, and option @option{--tmpdir} is +implied. + +When creating a file, the resulting file has read and write +permissions for the current user, but no permissions for the group or +others; these permissions are reduced if the current umask is more +restrictive. + +Here are some examples (although note that if you repeat them, you +will most likely get different file names): + +@itemize @bullet + +@item +Create a temporary file in the current directory. +@example +$ mktemp file.XXXX +file.H47c +@end example + +@item +Create a secure fifo relative to the user's choice of @env{TMPDIR}, +but falling back to the current directory rather than @file{/tmp}. +Note that @command{mktemp} does not create fifos, but can create a +secure directory in which the fifo can live. Exit the shell if the +directory or fifo could not be created. +@example +$ dir=$(mktemp -p "$@{TMPDIR:-.@}" -d dir-XXXX) || exit 1 +$ fifo=$dir/fifo +$ mkfifo "$fifo" || @{ rmdir $dir; exit 1; @} +@end example + +@item +Create and use a temporary file if possible, but ignore failure. The +file will reside in the directory named by @env{TMPDIR}, if specified, +or else in @file{/tmp}. +@example +$ file=$(mktemp -q) && @{ +> # Safe to use $file only within this block +> echo ... > $file +> rm $file +> @} +@end example + +@item +Act as a semi-random character generator (it is not fully random, +since it is impacted by the contents of the current directory). To +avoid security holes, do not use the resulting names to create a file. +@example +$ mktemp -u XXX +Gb9 +$ mktemp -u XXX +nzC +@end example + +@end itemize + + +The program accepts the following options. Also see @ref{Common options}. + +@table @samp + +@item -d +@itemx --directory +@opindex -d +@opindex --directory +Create a directory rather than a file. The directory will have read, +write, and execute permissions for the current user, but no +permissions for the group or others; these permissions are reduced if +the current umask is more restrictive. + +@item -q +@itemx --quiet +@opindex -q +@opindex --quiet +Suppress diagnostics about failure to create a file or directory. The +exit status will still reflect whether a file was created. + +@item -u +@itemx --dry-run +@opindex -u +@opindex --dry-run +Generate a temporary name that does not name an existing file, without +changing the file system contents. Using the output of this command +to create a new file is inherently unsafe, as there is a window of +time between generating the name and using it where another process +can create an object by the same name. + +@item -p @var{dir} +@itemx --tempdir[=@var{dir}] +@opindex -p +@opindex --tempdir +Treat @var{template} relative to the directory @var{dir}. If +@var{dir} is not specified (only possible with the long option +@option{--tempdir}) or is the empty string, use the value of +@env{TMPDIR} if available, otherwise use @samp{/tmp}. If this is +specified, @var{template} must not be absolute. However, +@var{template} can still contain slashes, although intermediate +directories must already exist. + +@item -t +@opindex -t +Treat @var{template} as a single file relative to the value of +@env{TMPDIR} if available, or to the directory specified by +@option{-p}, otherwise to @samp{/tmp}. @var{template} must not +contain slashes. This option is deprecated; the use of @option{-p} +without @option{-t} offers better defaults (by favoring the command +line over @env{TMPDIR}) and more flexibility (by allowing intermediate +directories). + +@end table + +@cindex exit status of @command{mktemp} +Exit status: + +@display +0 if the file was created, +1 otherwise. +@end display + @node Working context @chapter Working context -- 1.6.4.2 |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> Eric Blake <ebb9 <at> byu.net> writes: > >> Now that glibc 2.11 has mkostemps, and I'm working on adding that to gnulib, > it >> would be nice to expose the idea of an explicit suffix to temporary file >> names. But rather than require the user to count how long their suffix is, I >> imagine it would make more sense to give mktemp(1) some do-what-I-mean > smarts. Thanks a lot for writing the documentation (that's been a thorn in my side for far too long) and the careful once-over. I haven't had time yet to study much of this message, but bear in mind that I tried to implement a program that was faithful to this documentation: http://mktemp.org/manual.html I hope to get to this tomorrow. > Before I do that, how about documenting what mktemp currently does. The > proposed patch documents the current behavior, although it probably needs some > tweaks (and corresponding tweaks in mktemp.c) based this email. > > mktemp --help is not very obvious that --tmpdir is implied if no TEMPLATE is > given. |
|
|
Re: temp file suffixes: mktemp DWIMJim Meyering wrote:
> Eric Blake wrote: >> Eric Blake <ebb9 <at> byu.net> writes: >> >>> Now that glibc 2.11 has mkostemps, and I'm working on adding that to gnulib, >> it >>> would be nice to expose the idea of an explicit suffix to temporary file >>> names. But rather than require the user to count how long their suffix is, I >>> imagine it would make more sense to give mktemp(1) some do-what-I-mean >> smarts. > > Thanks a lot for writing the documentation (that's been a thorn in my > side for far too long) and the careful once-over. > I haven't had time yet to study much of this message, but bear > in mind that I tried to implement a program that was faithful > to this documentation: > > http://mktemp.org/manual.html Here's more perspective on -p vs -t: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/11527 |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi > index ec5bcfb..1fd1bec 100644 > --- a/doc/coreutils.texi > +++ b/doc/coreutils.texi > @@ -31,7 +31,6 @@ > @c FIXME: the following need documentation > @c * [: (coreutils)[ invocation. File/string tests. > @c * pinky: (coreutils)pinky invocation. FIXME. > -@c * mktemp: (coreutils)mktemp invocation. FIXME. There is also a mention in TODO to remove. The doc looks good from a very quick review, the examples are nice. cheers, Pádraig. |
|
|
Re: temp file suffixes: mktemp DWIMJim Meyering <jim <at> meyering.net> writes:
> > mktemp a => error, no run of X > > mktemp aXX => error, run of X is too short > > mktemp XXX => generates 3-character name (if possible) > > mktemp aXXX.b => generates 6-character name (if possible) > > mktemp --suffix=.b aXXX => longer spelling of the above line > > mktemp aXXX --suffix=.b => likewise, if not POSIXLY_CORRECT > > mktemp aXXXbXXX => generates aXXXb123 (only the last run changed) > > mktemp --suffix=X aXXX => only way to generate a123X instead of a1234 > > mktemp --suffix=.txt file => error, no run of X > > mktemp --suffix=.txt aXXXb => error, explicit --suffix requires trailing X > > Nice, comprehensive set of examples. Here's my first cut at implementing this. Eric Blake (4): [1/4] build: update gnulib submodule to latest, for tempname changes Real gnulib commit id TBD, once I push my pending mkostemps series to gnulib (http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/19282). Note that either patch 1 or patch 2 in isolation will cause bootstrap failure; so even though you normally like to do gnulib submodule updates in isolation, I'm wondering if this is a case where the two patches should be squashed together. [2/4] build: reflect gnulib changes to tempname Since gnulib added mkostemps support to gen_tempname, we need to reflect the extra parameter in gen_tempname_len. I think the .diff format makes it easier to see how we intentionally differ from glibc, rather than writing a flat-out replacement file (it's also more robust at catching subsequent gnulib changes to the same file, to let us know to resynchronize). [3/4] tests: enhance mktemp test Add more tests of existing behavior, to ensure I didn't break it, and to make it obvious what I'm changing. I could rebase the series to put this first, alongside my pending mktemp doc patch, especially if your review of my doc patch and the accompanying email turn up any changes we want to existing behavior. [4/4] mktemp: add suffix handling The real fun. Hopefully I added enough tests to cover everything new that results from the new code. Also, is the rearranged output in usage() okay, and should I split that into a separate patch? In rereading this before hitting the send button, I see an out-of-bounds reference in patch 4. I'll know if you reviewed this if you spot the same bug ;) Also, I guess I should also amend my series to mention this bug report in the commit message: > > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316 From 7a9f7441d27053c6842f1dc24087cd6bcca5e8b7 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 08:47:24 -0700 Subject: [PATCH 1/4] build: update gnulib submodule to latest, for tempname changes --- gnulib | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gnulib b/gnulib index 5ff8115..a820a49 160000 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 5ff811558adf7013f9fd9109fa794dd4e5ee8c91 +Subproject commit a820a49944558a7f956f1c6ed476fb1e90fff313 -- 1.6.4.2 From 4a45ad587212bc80315362fcb7aa19a6db714333 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 08:51:31 -0700 Subject: [PATCH 2/4] build: reflect gnulib changes to tempname * gl/lib/tempname.h: Change... * gl/lib/tempname.h.diff: ...to diff. Accommodate new suffixlen parameter. * gl/lib/tempname.c: Change... * gl/lib/tempname.c.diff: ...to diff. (check_x_suffix): Allow for X in suffix beyond x_suffix_len. (gen_tempname_len): Add suffixlen parameter. x_suffix_len is appropriately typed, but suffixlen inherits from the original botched type that glibc copied from BSD. (__gen_tempname): Update caller. * src/mktemp.c (mkstemp_len, mkdtemp_len): Update callers. --- gl/lib/tempname.c | 294 ------------------------------------------------ gl/lib/tempname.c.diff | 196 ++++++++++++++++++++++++++++++++ gl/lib/tempname.h | 42 ------- gl/lib/tempname.h.diff | 20 ++++ src/mktemp.c | 6 +- 5 files changed, 220 insertions(+), 338 deletions(-) delete mode 100644 gl/lib/tempname.c create mode 100644 gl/lib/tempname.c.diff delete mode 100644 gl/lib/tempname.h create mode 100644 gl/lib/tempname.h.diff diff --git a/gl/lib/tempname.c b/gl/lib/tempname.c deleted file mode 100644 index 84679bc..0000000 --- a/gl/lib/tempname.c +++ /dev/null @@ -1,294 +0,0 @@ -/* tempname.c - generate the name of a temporary file. - - Copyright (C) 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, - 2000, 2001, 2002, 2003, 2005, 2006, 2007, 2009 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 <http://www.gnu.org/licenses/>. */ - -/* Extracted from glibc sysdeps/posix/tempname.c. See also tmpdir.c. */ - -#if !_LIBC -# include <config.h> -# include "tempname.h" -# include "randint.h" -#endif - -#include <sys/types.h> -#include <assert.h> - -#include <errno.h> -#ifndef __set_errno -# define __set_errno(Val) errno = (Val) -#endif - -#include <stdio.h> -#ifndef P_tmpdir -# define P_tmpdir "/tmp" -#endif -#ifndef TMP_MAX -# define TMP_MAX 238328 -#endif -#ifndef __GT_FILE -# define __GT_FILE 1 -# define __GT_DIR 2 -# define __GT_NOCREATE 3 -#endif - -#include <stdbool.h> -#include <stddef.h> -#include <stdlib.h> -#include <string.h> - -#include <fcntl.h> -#include <sys/time.h> -#include <stdint.h> -#include <unistd.h> - -#include <sys/stat.h> - -#if _LIBC -# define struct_stat64 struct stat64 -#else -# define struct_stat64 struct stat -# define __open open -# define __gen_tempname gen_tempname -# define __getpid getpid -# define __gettimeofday gettimeofday -# define __mkdir mkdir -# define __lxstat64(version, file, buf) lstat (file, buf) -# define __xstat64(version, file, buf) stat (file, buf) -#endif - -#if ! (HAVE___SECURE_GETENV || _LIBC) -# define __secure_getenv getenv -#endif - -#if _LIBC -/* Return nonzero if DIR is an existent directory. */ -static int -direxists (const char *dir) -{ - struct_stat64 buf; - return __xstat64 (_STAT_VER, dir, &buf) == 0 && S_ISDIR (buf.st_mode); -} - -/* Path search algorithm, for tmpnam, tmpfile, etc. If DIR is - non-null and exists, uses it; otherwise uses the first of $TMPDIR, - P_tmpdir, /tmp that exists. Copies into TMPL a template suitable - for use with mk[s]temp. Will fail (-1) if DIR is non-null and - doesn't exist, none of the searched dirs exists, or there's not - enough space in TMPL. */ -int -__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, - int try_tmpdir) -{ - const char *d; - size_t dlen, plen; - - if (!pfx || !pfx[0]) - { - pfx = "file"; - plen = 4; - } - else - { - plen = strlen (pfx); - if (plen > 5) - plen = 5; - } - - if (try_tmpdir) - { - d = __secure_getenv ("TMPDIR"); - if (d != NULL && direxists (d)) - dir = d; - else if (dir != NULL && direxists (dir)) - /* nothing */ ; - else - dir = NULL; - } - if (dir == NULL) - { - if (direxists (P_tmpdir)) - dir = P_tmpdir; - else if (strcmp (P_tmpdir, "/tmp") != 0 && direxists ("/tmp")) - dir = "/tmp"; - else - { - __set_errno (ENOENT); - return -1; - } - } - - dlen = strlen (dir); - while (dlen > 1 && dir[dlen - 1] == '/') - dlen--; /* remove trailing slashes */ - - /* check we have room for "${dir}/${pfx}XXXXXX\0" */ - if (tmpl_len < dlen + 1 + plen + 6 + 1) - { - __set_errno (EINVAL); - return -1; - } - - sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx); - return 0; -} -#endif /* _LIBC */ - -static inline bool -check_x_suffix (char const *s, size_t len) -{ - return strspn (s, "X") == len; -} - -/* These are the characters used in temporary file names. */ -static const char letters[] = -"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; - -/* Generate a temporary file name based on TMPL. TMPL must end in a - a sequence of at least X_SUFFIX_LEN "X"s. The name constructed - does not exist at the time of the call to __gen_tempname. TMPL is - overwritten with the result. - - KIND may be one of: - __GT_NOCREATE: simply verify that the name does not exist - at the time of the call. - __GT_FILE: create the file using open(O_CREAT|O_EXCL) - and return a read-write fd. The file is mode 0600. - __GT_DIR: create a directory, which will be mode 0700. - - We use a clever algorithm to get hard-to-predict names. */ -int -gen_tempname_len (char *tmpl, int flags, int kind, size_t x_suffix_len) -{ - size_t len; - char *XXXXXX; - unsigned int count; - int fd = -1; - int save_errno = errno; - struct_stat64 st; - struct randint_source *rand_src; - - /* A lower bound on the number of temporary files to attempt to - generate. The maximum total number of temporary file names that - can exist for a given template is 62**6. It should never be - necessary to try all these combinations. Instead if a reasonable - number of names is tried (we define reasonable as 62**3) fail to - give the system administrator the chance to remove the problems. */ -#define ATTEMPTS_MIN (62 * 62 * 62) - - /* The number of times to attempt to generate a temporary file. To - conform to POSIX, this must be no smaller than TMP_MAX. */ -#if ATTEMPTS_MIN < TMP_MAX - unsigned int attempts = TMP_MAX; -#else - unsigned int attempts = ATTEMPTS_MIN; -#endif - - len = strlen (tmpl); - if (len < x_suffix_len || ! check_x_suffix (&tmpl[len - x_suffix_len], - x_suffix_len)) - { - __set_errno (EINVAL); - return -1; - } - - rand_src = randint_all_new (NULL, 8); - if (! rand_src) - return -1; - - /* This is where the Xs start. */ - XXXXXX = &tmpl[len - x_suffix_len]; - - for (count = 0; count < attempts; ++count) - { - size_t i; - - for (i = 0; i < x_suffix_len; i++) - { - XXXXXX[i] = letters[randint_genmax (rand_src, sizeof letters - 2)]; - } - - switch (kind) - { - case __GT_FILE: - fd = __open (tmpl, - (flags & ~0777) | O_RDWR | O_CREAT | O_EXCL, - S_IRUSR | S_IWUSR); - break; - - case __GT_DIR: - fd = __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR); - break; - - case __GT_NOCREATE: - /* This case is backward from the other three. This function - succeeds if __xstat fails because the name does not exist. - Note the continue to bypass the common logic at the bottom - of the loop. */ - if (__lxstat64 (_STAT_VER, tmpl, &st) < 0) - { - if (errno == ENOENT) - { - __set_errno (save_errno); - fd = 0; - goto done; - } - else - { - /* Give up now. */ - fd = -1; - goto done; - } - } - continue; - - default: - assert (! "invalid KIND in __gen_tempname"); - } - - if (fd >= 0) - { - __set_errno (save_errno); - goto done; - } - else if (errno != EEXIST) - { - fd = -1; - goto done; - } - } - - randint_all_free (rand_src); - - /* We got out of the loop because we ran out of combinations to try. */ - __set_errno (EEXIST); - return -1; - - done: - { - int saved_errno = errno; - randint_all_free (rand_src); - __set_errno (saved_errno); - } - return fd; -} - -int -__gen_tempname (char *tmpl, int flags, int kind) -{ - return gen_tempname_len (tmpl, flags, kind, 6); -} diff --git a/gl/lib/tempname.c.diff b/gl/lib/tempname.c.diff new file mode 100644 index 0000000..e00e120 --- /dev/null +++ b/gl/lib/tempname.c.diff @@ -0,0 +1,196 @@ +diff --git i/lib/tempname.c w/lib/tempname.c +index 2da5afe..b892b66 100644 +--- i/lib/tempname.c ++++ w/lib/tempname.c +@@ -22,6 +22,7 @@ + #if !_LIBC + # include <config.h> + # include "tempname.h" ++# include "randint.h" + #endif + + #include <sys/types.h> +@@ -49,6 +50,7 @@ + # error report this to bug-gnulib@... + #endif + ++#include <stdbool.h> + #include <stddef.h> + #include <stdlib.h> + #include <string.h> +@@ -179,14 +181,21 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, + } + #endif /* _LIBC */ + ++static inline bool ++check_x_suffix (char const *s, size_t len) ++{ ++ return len <= strspn (s, "X"); ++} ++ + /* These are the characters used in temporary file names. */ + static const char letters[] = + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + + /* Generate a temporary file name based on TMPL. TMPL must match the +- rules for mk[s]temp (i.e. end in "XXXXXX", possibly with a suffix). +- The name constructed does not exist at the time of the call to +- __gen_tempname. TMPL is overwritten with the result. ++ rules for mk[s]temp (i.e. end in at least X_SUFFIX_LEN "X"s, ++ possibly with a suffix). The name constructed does not exist at ++ the time of the call to this function. TMPL is overwritten with ++ the result. + + KIND may be one of: + __GT_NOCREATE: simply verify that the name does not exist +@@ -197,23 +206,24 @@ static const char letters[] = + + We use a clever algorithm to get hard-to-predict names. */ + int +-__gen_tempname (char *tmpl, int suffixlen, int flags, int kind) ++gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind, ++ size_t x_suffix_len) + { +- int len; ++ size_t len; + char *XXXXXX; +- static uint64_t value; +- uint64_t random_time_bits; + unsigned int count; + int fd = -1; + int save_errno = errno; + struct_stat64 st; ++ struct randint_source *rand_src; + + /* A lower bound on the number of temporary files to attempt to + generate. The maximum total number of temporary file names that + can exist for a given template is 62**6. It should never be + necessary to try all these combinations. Instead if a reasonable + number of names is tried (we define reasonable as 62**3) fail to +- give the system administrator the chance to remove the problems. */ ++ give the system administrator the chance to remove the problems. ++ This value requires that X_SUFFIX_LEN be at least 3. */ + #define ATTEMPTS_MIN (62 * 62 * 62) + + /* The number of times to attempt to generate a temporary file. To +@@ -225,43 +235,30 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) + #endif + + len = strlen (tmpl); +- if (len < 6 + suffixlen || memcmp (&tmpl[len - 6 - suffixlen], "XXXXXX", 6)) ++ if (len < x_suffix_len + suffixlen ++ || ! check_x_suffix (&tmpl[len - x_suffix_len - suffixlen], ++ x_suffix_len)) + { + __set_errno (EINVAL); + return -1; + } + + /* This is where the Xs start. */ +- XXXXXX = &tmpl[len - 6 - suffixlen]; ++ XXXXXX = &tmpl[len - x_suffix_len - suffixlen]; + + /* Get some more or less random data. */ +-#ifdef RANDOM_BITS +- RANDOM_BITS (random_time_bits); +-#else +- { +- struct timeval tv; +- __gettimeofday (&tv, NULL); +- random_time_bits = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec; +- } +-#endif +- value += random_time_bits ^ __getpid (); ++ rand_src = randint_all_new (NULL, 8); ++ if (! rand_src) ++ return -1; + +- for (count = 0; count < attempts; value += 7777, ++count) ++ for (count = 0; count < attempts; ++count) + { +- uint64_t v = value; +- +- /* Fill in the random bits. */ +- XXXXXX[0] = letters[v % 62]; +- v /= 62; +- XXXXXX[1] = letters[v % 62]; +- v /= 62; +- XXXXXX[2] = letters[v % 62]; +- v /= 62; +- XXXXXX[3] = letters[v % 62]; +- v /= 62; +- XXXXXX[4] = letters[v % 62]; +- v /= 62; +- XXXXXX[5] = letters[v % 62]; ++ size_t i; ++ ++ for (i = 0; i < x_suffix_len; i++) ++ { ++ XXXXXX[i] = letters[randint_genmax (rand_src, sizeof letters - 2)]; ++ } + + switch (kind) + { +@@ -276,7 +273,7 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) + break; + + case __GT_NOCREATE: +- /* This case is backward from the other three. __gen_tempname ++ /* This case is backward from the other three. This function + succeeds if __xstat fails because the name does not exist. + Note the continue to bypass the common logic at the bottom + of the loop. */ +@@ -285,11 +282,15 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) + if (errno == ENOENT) + { + __set_errno (save_errno); +- return 0; ++ fd = 0; ++ goto done; + } + else +- /* Give up now. */ +- return -1; ++ { ++ /* Give up now. */ ++ fd = -1; ++ goto done; ++ } + } + continue; + +@@ -301,13 +302,32 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) + if (fd >= 0) + { + __set_errno (save_errno); +- return fd; ++ goto done; + } + else if (errno != EEXIST) +- return -1; ++ { ++ fd = -1; ++ goto done; ++ } + } + ++ randint_all_free (rand_src); ++ + /* We got out of the loop because we ran out of combinations to try. */ + __set_errno (EEXIST); + return -1; ++ ++ done: ++ { ++ int saved_errno = errno; ++ randint_all_free (rand_src); ++ __set_errno (saved_errno); ++ } ++ return fd; ++} ++ ++int ++__gen_tempname (char *tmpl, int suffixlen, int flags, int kind) ++{ ++ return gen_tempname_len (tmpl, suffixlen, flags, kind, 6); + } diff --git a/gl/lib/tempname.h b/gl/lib/tempname.h deleted file mode 100644 index a942f07..0000000 --- a/gl/lib/tempname.h +++ /dev/null @@ -1,42 +0,0 @@ -/* Create a temporary file or directory. - - Copyright (C) 2006, 2007, 2009 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 <http://www.gnu.org/licenses/>. */ - -/* header written by Eric Blake */ - -/* In gnulib, always prefer large files. GT_FILE maps to - __GT_BIGFILE, not __GT_FILE, for a reason. */ -#define GT_FILE 1 -#define GT_DIR 2 -#define GT_NOCREATE 3 - -/* Generate a temporary file name based on TMPL. TMPL must match the - rules for mk[s]temp (i.e. end in "XXXXXX"). The name constructed - does not exist at the time of the call to gen_tempname. TMPL is - overwritten with the result. - - KIND may be one of: - GT_NOCREATE: simply verify that the name does not exist - at the time of the call. - GT_FILE: create a large file using open(O_CREAT|O_EXCL) - and return a read-write fd. The file is mode 0600. - GT_DIR: create a directory, which will be mode 0700. - - We use a clever algorithm to get hard-to-predict names. */ -#include <stddef.h> -extern int gen_tempname (char *tmpl, int flags, int kind); -extern int gen_tempname_len (char *tmpl, int flags, int kind, - size_t x_suffix_len); diff --git a/gl/lib/tempname.h.diff b/gl/lib/tempname.h.diff new file mode 100644 index 0000000..696b290 --- /dev/null +++ b/gl/lib/tempname.h.diff @@ -0,0 +1,20 @@ +diff --git i/lib/tempname.h w/lib/tempname.h +index 6417df2..25286ec 100644 +--- i/lib/tempname.h ++++ w/lib/tempname.h +@@ -1,6 +1,6 @@ + /* Create a temporary file or directory. + +- Copyright (C) 2006, 2009 Free Software Foundation, Inc. ++ Copyright (C) 2006, 2007, 2009 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 +@@ -46,5 +46,7 @@ + + We use a clever algorithm to get hard-to-predict names. */ + extern int gen_tempname (char *tmpl, int suffixlen, int flags, int kind); ++extern int gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind, ++ size_t x_suffix_len); + + #endif GL_TEMPNAME_H diff --git a/src/mktemp.c b/src/mktemp.c index 808efa9..fc123b6 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -114,13 +114,15 @@ count_trailing_X_s (const char *s) static int mkstemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_FILE, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_FILE, + suff_len); } static int mkdtemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_DIR, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_DIR, + suff_len); } int -- 1.6.4.2 From 2109f145fd3cac264080dde0892eefe9850d0f00 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 09:01:49 -0700 Subject: [PATCH 3/4] tests: enhance mktemp test * tests/misc/mktemp: Add more coverage. --- tests/misc/mktemp | 28 +++++++++++++++++++++++++++- 1 files changed, 27 insertions(+), 1 deletions(-) diff --git a/tests/misc/mktemp b/tests/misc/mktemp index dde1532..e620311 100755 --- a/tests/misc/mktemp +++ b/tests/misc/mktemp @@ -62,6 +62,7 @@ my @Tests = ['too-few-x', 'foo.XX', {ERR=>"$prog: too few X's in template `foo.XX'\n"}, {EXIT => 1} ], + ['too-few-xq', '-q foo.XX', {EXIT => 1} ], ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, @@ -69,6 +70,12 @@ my @Tests = check_tmp $f, 'F'; }} ], + ['2f', '-- -XXXX', {OUT => "-ZZZZ\n"}, + {OUT_SUBST => 's,-....$,-ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }} + ], + # Create a temporary directory. ['1d', '-d f.XXXX', {OUT => "f.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, @@ -83,7 +90,18 @@ my @Tests = check_tmp $f, 'D'; }} ], - ['invalid-tmpl', '-t a/bXXXX', + # Test -u + ['uf', '-u f.XXXX', {OUT => "f.ZZZZ\n"}, + {OUT_SUBST => 's,\.....$,.ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created file"; }}], + ['ud', '-d --dry-run d.XXXX', {OUT => "d.ZZZZ\n"}, + {OUT_SUBST => 's,\.....$,.ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created directory"; }}], + + # Test bad templates + ['invalid-tl', '-t a/bXXXX', {ERR=>"$prog: invalid template, `a/bXXXX', " . "contains directory separator\n"}, {EXIT => 1} ], @@ -91,6 +109,11 @@ my @Tests = {ERR=>"$prog: invalid template, `/bXXXX'; " . "with --tmpdir, it may not be absolute\n"}, {EXIT => 1} ], + # Suffix after X. + ['invalid-t3', 'aXXXXb', + {ERR=>"$prog: too few X's in template `aXXXXb'\n"}, {EXIT => 1} ], + + # Test template with subdirectory ['tmp-w-slash', '--tmpdir=. a/bXXXX', {PRE => sub {mkdir 'a',0755 or die "a: $!\n"}}, {OUT_SUBST => 's,b....$,bZZZZ,'}, @@ -104,6 +127,9 @@ my @Tests = {ERR_SUBST => "s,($bad_dir/)[^']+': .*,\$1...,"}, {ERR => "$prog: failed to create file via template `$bad_dir/...\n"}, {EXIT => 1}], + ['pipe-bad-tmpdir-u', '-u', {OUT => "$bad_dir/tmp.ZZZZZZZZZZ\n"}, + {ENV => "TMPDIR=$bad_dir"}, + {OUT_SUBST => 's,\..{10}$,.ZZZZZZZZZZ,'}], ); my $save_temps = $ENV{DEBUG}; -- 1.6.4.2 From 97a265760543b4ea6ab5dd1aa205bcd9fd8544d0 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 11:13:39 -0700 Subject: [PATCH 4/4] mktemp: add suffix handling * src/mktemp.c (TMPDIR_OPTION): New enum value. (longopts): Add new option. (usage): Document it. Reformat other options. (count_trailing_X_s): Rename... (count_consecutive_X_s): ...to this, and add parameter. (mkstemp_len, mkdtemp_len): Add parameter. (main): Implement new option. (AUTHORS): Add myself. * AUTHORS (mktemp): Likewise. * tests/misc/mktemp: Test new option. * doc/coreutils.texi (mktemp invocation): Document it. * NEWS: Likewise. --- AUTHORS | 2 +- NEWS | 4 ++ doc/coreutils.texi | 25 ++++++++++- src/mktemp.c | 111 ++++++++++++++++++++++++++++++++++++---------------- tests/misc/mktemp | 70 +++++++++++++++++++++++++++++++-- 5 files changed, 170 insertions(+), 42 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7095db0..59a0dfa 100644 --- a/AUTHORS +++ b/AUTHORS @@ -46,7 +46,7 @@ md5sum: Ulrich Drepper, Scott Miller, David Madore mkdir: David MacKenzie mkfifo: David MacKenzie mknod: David MacKenzie -mktemp: Jim Meyering +mktemp: Jim Meyering, Eric Blake mv: Mike Parker, David MacKenzie, Jim Meyering nice: David MacKenzie nl: Scott Bartram, David MacKenzie diff --git a/NEWS b/NEWS index 03ed83f..3f8baf0 100644 --- a/NEWS +++ b/NEWS @@ -63,6 +63,10 @@ GNU coreutils NEWS -*- outline -*- md5sum --check now also accepts openssl-style checksums. So do sha1sum, sha224sum, sha384sum and sha512sum. + mktemp now accepts the option --suffix to provide a known suffix + after the substitution in the template. Additionally, uses such as + "mktemp fileXXXXXX.txt" are able to infer an appropriate --suffix. + touch now accepts the option --no-dereference (-h), as a means to change symlink timestamps on platforms with enough support. diff --git a/doc/coreutils.texi b/doc/coreutils.texi index e45ecff..b457f88 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -12068,10 +12068,10 @@ mktemp invocation @end example Safely create a temporary file or directory based on @var{template}, -and print its name. If given, @var{template} must end in at least -three consecutive @samp{X}. If omitted, the template +and print its name. If given, @var{template} must include at least +three consecutive @samp{X} in the last component. If omitted, the template @samp{tmp.XXXXXXXXXX} is used, and option @option{--tmpdir} is -implied. The trailing @samp{X} in the @var{template} will be replaced +implied. The final run of @samp{X} in the @var{template} will be replaced by alpha-numeric characters; thus, on a case-sensitive file system, and with a @var{template} ending in @var{n} instances of @samp{X}, there are @samp{62**@var{n}} potential file names. @@ -12108,6 +12108,15 @@ mktemp invocation @end example @item +Create a temporary file with a known suffix. +@example +$ mktemp --suffix=.txt file-XXXX +file-H08W.txt +$ mktemp file-XXXX-XXXX.txt +file-XXXX-eI9L.txt +@end example + +@item Create a secure fifo relative to the user's choice of @env{TMPDIR}, but falling back to the current directory rather than @file{/tmp}. Note that @command{mktemp} does not create fifos, but can create a @@ -12186,6 +12195,16 @@ mktemp invocation @var{template} can still contain slashes, although intermediate directories must already exist. +@item --suffix=@var{suffix} +@opindex --suffix +Append @var{suffix} to the @var{template}. @var{suffix} must not +contain slash. If @option{--suffix} is specified, @var{template} must +end in @samp{X}; if it is not specified, then an appropriate +@option{--suffix} is inferred by finding the last @samp{X} in +@var{template}. This option exists for use with the default +@var{template} and for the creation of a @var{suffix} that starts with +@samp{X}. + @item -t @opindex -t Treat @var{template} as a single file relative to the value of diff --git a/src/mktemp.c b/src/mktemp.c index fc123b6..da50165 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -14,7 +14,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ -/* Written by Jim Meyering. */ +/* Written by Jim Meyering and Eric Blake. */ #include <config.h> #include <stdio.h> @@ -31,7 +31,9 @@ /* The official name of this program (e.g., no `g' prefix). */ #define PROGRAM_NAME "mktemp" -#define AUTHORS proper_name ("Jim Meyering") +#define AUTHORS \ + proper_name ("Jim Meyering"), \ + proper_name ("Eric Blake") static const char *default_template = "tmp.XXXXXXXXXX"; @@ -39,7 +41,8 @@ static const char *default_template = "tmp.XXXXXXXXXX"; non-character as a pseudo short option, starting with CHAR_MAX + 1. */ enum { - TMPDIR_OPTION = CHAR_MAX + 1 + SUFFIX_OPTION = CHAR_MAX + 1, + TMPDIR_OPTION }; static struct option const longopts[] = @@ -47,6 +50,7 @@ static struct option const longopts[] = {"directory", no_argument, NULL, 'd'}, {"quiet", no_argument, NULL, 'q'}, {"dry-run", no_argument, NULL, 'u'}, + {"suffix", required_argument, NULL, SUFFIX_OPTION}, {"tmpdir", optional_argument, NULL, TMPDIR_OPTION}, {GETOPT_HELP_OPTION_DECL}, {GETOPT_VERSION_OPTION_DECL}, @@ -64,33 +68,34 @@ usage (int status) printf (_("Usage: %s [OPTION]... [TEMPLATE]\n"), program_name); fputs (_("\ Create a temporary file or directory, safely, and print its name.\n\ +TEMPLATE must contain at least 3 consecutive X in last component.\n\ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ "), stdout); fputs ("\n", stdout); fputs (_("\ - -d, --directory create a directory, not a file\n\ + -d, --directory create a directory, not a file\n\ + -u, --dry-run do not create anything; merely print a name (unsafe)\n\ + -q, --quiet suppress diagnostics about file/dir-creation failure\n\ "), stdout); fputs (_("\ - -q, --quiet suppress diagnostics about file/dir-creation failure\n\ + --suffix=SUFF append SUFF to TEMPLATE. SUFF must not contain slash.\n\ + This option is implied if TEMPLATE does not end in X.\n\ "), stdout); fputs (_("\ - -u, --dry-run do not create anything; merely print a name (unsafe)\n\ -"), stdout); - fputs (_("\ - --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is\n\ - not specified, use $TMPDIR if set, else /tmp.\n\ - With this option, TEMPLATE must not be an absolute name.\n\ - Unlike with -t, TEMPLATE may contain slashes, but even\n\ - here, mktemp still creates only the final component.\n\ + --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is not\n\ + specified, use $TMPDIR if set, else /tmp. With\n\ + this option, TEMPLATE must not be an absolute name.\n\ + Unlike with -t, TEMPLATE may contain slashes, but\n\ + mktemp creates only the final component.\n\ "), stdout); fputs ("\n", stdout); fputs (_("\ - -p DIR use DIR as a prefix; implies -t [deprecated]\n\ + -p DIR use DIR as a prefix; implies -t [deprecated]\n\ "), stdout); fputs (_("\ - -t interpret TEMPLATE as a single file name component,\n\ - relative to a directory: $TMPDIR, if set; else the\n\ - directory specified via -p; else /tmp [deprecated]\n\ + -t interpret TEMPLATE as a single file name component,\n\ + relative to a directory: $TMPDIR, if set; else the\n\ + directory specified via -p; else /tmp [deprecated]\n\ "), stdout); fputs ("\n", stdout); fputs (HELP_OPTION_DESCRIPTION, stdout); @@ -102,9 +107,8 @@ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ } static size_t -count_trailing_X_s (const char *s) +count_consecutive_X_s (const char *s, size_t len) { - size_t len = strlen (s); size_t n = 0; for ( ; len && s[len-1] == 'X'; len--) ++n; @@ -112,17 +116,17 @@ count_trailing_X_s (const char *s) } static int -mkstemp_len (char *tmpl, size_t suff_len, bool dry_run) +mkstemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_FILE, - suff_len); + return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_FILE, + x_len); } static int -mkdtemp_len (char *tmpl, size_t suff_len, bool dry_run) +mkdtemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_DIR, - suff_len); + return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_DIR, + x_len); } int @@ -134,12 +138,14 @@ main (int argc, char **argv) int c; unsigned int n_args; char *template; + char *suffix = NULL; bool use_dest_dir = false; bool deprecated_t_option = false; bool create_directory = false; bool dry_run = false; int status = EXIT_SUCCESS; size_t x_count; + size_t suffix_len; char *dest_name; initialize_main (&argc, &argv); @@ -177,9 +183,13 @@ main (int argc, char **argv) dest_dir_arg = optarg; break; + case SUFFIX_OPTION: + suffix = optarg; + break; + case_GETOPT_HELP_CHAR; - case 'V': + case 'V': /* Undocumented alias. FIXME: remove in 2011. */ case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); default: usage (EXIT_FAILURE); @@ -212,7 +222,41 @@ main (int argc, char **argv) template = argv[optind]; } - x_count = count_trailing_X_s (template); + if (suffix) + { + size_t len = strlen (template); + if (template[len - 1] != 'X') + { + error (EXIT_FAILURE, 0, + _("with --suffix, template %s must end in X"), + quote (template)); + } + suffix_len = strlen (suffix); + dest_name = xcharalloc (len + suffix_len + 1); + memcpy (dest_name, template, len); + memcpy (dest_name + len, suffix, suffix_len + 1); + template = dest_name; + suffix = dest_name + len; + } + else + { + template = xstrdup (template); + suffix = strrchr (template, 'X'); + if (!suffix) + suffix = strchr (template, '\0'); + else + suffix++; + suffix_len = strlen (suffix); + } + + /* At this point, template is malloc'd, and suffix points into template. */ + if (suffix_len && last_component (suffix) != suffix) + { + error (EXIT_FAILURE, 0, + _("invalid suffix %s, contains directory separator"), + quote (suffix)); + } + x_count = count_consecutive_X_s (template, suffix - template); if (x_count < 3) error (EXIT_FAILURE, 0, _("too few X's in template %s"), quote (template)); @@ -246,11 +290,10 @@ main (int argc, char **argv) quote (template)); } - template = file_name_concat (dest_dir, template, NULL); - } - else - { - template = xstrdup (template); + dest_name = file_name_concat (dest_dir, template, NULL); + free (template); + template = dest_name; + /* Note that suffix is now invalid. */ } /* Make a copy to be used in case of diagnostic, since failing @@ -259,7 +302,7 @@ main (int argc, char **argv) if (create_directory) { - int err = mkdtemp_len (dest_name, x_count, dry_run); + int err = mkdtemp_len (dest_name, suffix_len, x_count, dry_run); if (err != 0) { error (0, errno, _("failed to create directory via template %s"), @@ -269,7 +312,7 @@ main (int argc, char **argv) } else { - int fd = mkstemp_len (dest_name, x_count, dry_run); + int fd = mkstemp_len (dest_name, suffix_len, x_count, dry_run); if (fd < 0 || (!dry_run && close (fd) != 0)) { error (0, errno, _("failed to create file via template %s"), diff --git a/tests/misc/mktemp b/tests/misc/mktemp index e620311..9cb27ed 100755 --- a/tests/misc/mktemp +++ b/tests/misc/mktemp @@ -60,8 +60,8 @@ my @Tests = . "Try `$prog --help' for more information.\n"}, {EXIT => 1} ], ['too-many-q', '-q a b', {EXIT => 1} ], - ['too-few-x', 'foo.XX', - {ERR=>"$prog: too few X's in template `foo.XX'\n"}, {EXIT => 1} ], + ['too-few-x', 'foo.XX', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `foo.XX'\n"}], ['too-few-xq', '-q foo.XX', {EXIT => 1} ], ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, @@ -110,8 +110,70 @@ my @Tests = . "with --tmpdir, it may not be absolute\n"}, {EXIT => 1} ], # Suffix after X. - ['invalid-t3', 'aXXXXb', - {ERR=>"$prog: too few X's in template `aXXXXb'\n"}, {EXIT => 1} ], + ['suffix1f', 'aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + ['suffix1d', '-d aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'D'; }}], + ['suffix1u', '-u aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created file"; }}], + + ['suffix2f', 'aXXXXaaXXXXa', {OUT=>"aXXXXaaZZZZa\n"}, + {OUT_SUBST=>'s,a....a$,aZZZZa,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + ['suffix2d', '-d --suffix= aXXXXaaXXXX', {OUT=>"aXXXXaaZZZZ\n"}, + {OUT_SUBST=>'s,a....$,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'D'; }}], + + ['suffix3f', '--suffix=b aXXXX', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix4f', '--suffix=X aXXXX', {OUT=>"aZZZZX\n"}, + {OUT_SUBST=>'s,^a....,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix5f', '--suffix /b aXXXX', {EXIT=>1}, + {ERR=>"$prog: invalid suffix `/b', contains directory separator\n"}], + + ['suffix6f', 'aXXXX/b', {EXIT=>1}, + {ERR=>"$prog: invalid suffix `/b', contains directory separator\n"}], + ['suffix6f-q', '-q aXXXX/b', {EXIT=>1}], + + ['suffix7f', '--suffix= aXXXXb', {EXIT=>1}, + {ERR=>"$prog: with --suffix, template `aXXXXb' must end in X\n"}], + ['suffix7f-q', '-q --suffix= aXXXXb', {EXIT=>1}], + + ['suffix8f', 'aXXXX --suffix=b', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,^a....,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix9f', 'aXXXX --suffix=b', {EXIT=>1}, + {ENV=>"POSIXLY_CORRECT=1"}, + {ERR=>"$prog: too many templates\n" + . "Try `$prog --help' for more information.\n"}], + + ['suffix10f', 'aXXb', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `aXXb'\n"}], + ['suffix10d', '-d --suffix=X aXX', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `aXXX'\n"}], + + ['suffix11f', '--suffix=.txt', {OUT=>"./tmp.ZZZZZZZZZZ.txt\n"}, + {ENV=>"TMPDIR=."}, + {OUT_SUBST=>'s,\..{10}\.,.ZZZZZZZZZZ.,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + # Test template with subdirectory ['tmp-w-slash', '--tmpdir=. a/bXXXX', -- 1.6.4.2 |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake <ebb9 <at> byu.net> writes:
> This currently gives exit status 1, because the atexit() handler recognizes > failure to print the file name to stdout, but leaves the temporary file > around. Should we go ahead and manually flush/close stdout, rather than > relying on close_stdout, so that we can then remove the just-created file if we > detect write failure? That way, if we fail to inform the user what just got > created, we are at least avoiding littering their file system with a random > file. > > Fortunately, 'mktemp >&-' does not make the mistake of printing the just- > created file as the contents of that file, since we close the fd returned by > mkstemp before printing anything. How about the following? From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 14:02:20 -0700 Subject: [PATCH] mktemp: don't leave file behind on write failure * src/mktemp.c (main): Remove just-created file if stdout had problems. * bootstrap.conf (gnulib_modules): Add remove. * tests/misc/close-stdout: Test it. * NEWS: Document it. --- NEWS | 4 ++++ bootstrap.conf | 1 + src/mktemp.c | 12 +++++++++++- tests/misc/close-stdout | 6 ++++++ 4 files changed, 22 insertions(+), 1 deletions(-) diff --git a/NEWS b/NEWS index 3f8baf0..4d1b4f5 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,10 @@ GNU coreutils NEWS -*- outline -*- This also affected sum, sha1sum, sha224sum, sha384sum and sha512sum. [the bug dates back to the initial implementation] + mktemp no longer leaves a temporary file behind if it was unable to + output the name of the file to stdout. + [the bug dates back to the initial implementation] + nice -n -1 PROGRAM now runs PROGRAM even when its internal setpriority call fails with errno == EACCES. [the bug dates back to the initial implementation] diff --git a/bootstrap.conf b/bootstrap.conf index 7960546..b3a82e0 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -180,6 +180,7 @@ gnulib_modules=" readutmp realloc regex + remove rename rmdir root-dev-ino diff --git a/src/mktemp.c b/src/mktemp.c index e645362..12acad8 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -23,6 +23,7 @@ #include "system.h" +#include "close-stream.h" #include "error.h" #include "filenamecat.h" #include "quote.h" @@ -322,7 +323,16 @@ main (int argc, char **argv) } if (status == EXIT_SUCCESS) - puts (dest_name); + { + puts (dest_name); + /* If we created a file, but then failed to output the file + name, we should clean up the mess before failing. */ + if (!dry_run && close_stream (stdout)) + { + remove (dest_name); + error (EXIT_FAILURE, errno, _("write error")); + } + } #ifdef lint free (dest_name); diff --git a/tests/misc/close-stdout b/tests/misc/close-stdout index fec1add..852c3c8 100755 --- a/tests/misc/close-stdout +++ b/tests/misc/close-stdout @@ -50,12 +50,18 @@ if "$p/src/test" -w /dev/stdout >/dev/null && "$p/src/test" ! -w /dev/stdout >&-; then "$p/src/printf" 'foo' >&- 2>/dev/null && fail=1 cp --verbose a b >&- 2>/dev/null && fail=1 + rm -Rf tmpfile-?????? || fail=1 + mktemp tmpfile-XXXXXX >&- 2>/dev/null && fail=1 + test -e tmpfile-?????? && fail=1 fi # Likewise for /dev/full, if /dev/full works. if test -w /dev/full && test -c /dev/full; then "$p/src/printf" 'foo' >/dev/full 2>/dev/null && fail=1 cp --verbose a b >/dev/full 2>/dev/null && fail=1 + rm -Rf tmpdir-?????? || fail=1 + mktemp -d tmpdir-XXXXXX >/dev/full 2>/dev/null && fail=1 + test -e tmpdir-?????? && fail=1 fi Exit $fail -- 1.6.4.2 |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake <ebb9 <at> byu.net> writes:
> if (status == EXIT_SUCCESS) > - puts (dest_name); > + { > + puts (dest_name); > + /* If we created a file, but then failed to output the file > + name, we should clean up the mess before failing. */ > + if (!dry_run && close_stream (stdout)) > + { > + remove (dest_name); > + error (EXIT_FAILURE, errno, _("write error")); The remove() can corrupt errno, so I'm squashing this on top: diff --git i/src/mktemp.c w/src/mktemp.c index 12acad8..049401f 100644 --- i/src/mktemp.c +++ w/src/mktemp.c @@ -329,8 +329,9 @@ main (int argc, char **argv) name, we should clean up the mess before failing. */ if (!dry_run && close_stream (stdout)) { + int saved_errno = errno; remove (dest_name); - error (EXIT_FAILURE, errno, _("write error")); + error (EXIT_FAILURE, saved_errno, _("write error")); } } -- Eric Blake |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> Eric Blake <ebb9 <at> byu.net> writes: > >> This currently gives exit status 1, because the atexit() handler recognizes >> failure to print the file name to stdout, but leaves the temporary file >> around. Should we go ahead and manually flush/close stdout, rather than >> relying on close_stdout, so that we can then remove the just-created file if > we >> detect write failure? That way, if we fail to inform the user what just got >> created, we are at least avoiding littering their file system with a random >> file. >> >> Fortunately, 'mktemp >&-' does not make the mistake of printing the just- >> created file as the contents of that file, since we close the fd returned by >> mkstemp before printing anything. > > How about the following? > > From: Eric Blake <ebb9@...> > Date: Wed, 4 Nov 2009 14:02:20 -0700 > Subject: [PATCH] mktemp: don't leave file behind on write failure > > * src/mktemp.c (main): Remove just-created file if stdout had > problems. > * bootstrap.conf (gnulib_modules): Add remove. > * tests/misc/close-stdout: Test it. > * NEWS: Document it. With your fix-up, that's a fine fix. Thanks! I should catch up on your doc- and primary change tomorrow. |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> Jim Meyering <jim <at> meyering.net> writes: > >> > mktemp a => error, no run of X >> > mktemp aXX => error, run of X is too short >> > mktemp XXX => generates 3-character name (if possible) >> > mktemp aXXX.b => generates 6-character name (if possible) >> > mktemp --suffix=.b aXXX => longer spelling of the above line >> > mktemp aXXX --suffix=.b => likewise, if not POSIXLY_CORRECT >> > mktemp aXXXbXXX => generates aXXXb123 (only the last run changed) >> > mktemp --suffix=X aXXX => only way to generate a123X instead of a1234 >> > mktemp --suffix=.txt file => error, no run of X >> > mktemp --suffix=.txt aXXXb => error, explicit --suffix requires trailing X >> >> Nice, comprehensive set of examples. > > Here's my first cut at implementing this. > > Eric Blake (4): > [1/4] build: update gnulib submodule to latest, for tempname changes > Real gnulib commit id TBD, once I push my pending mkostemps series to gnulib > (http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/19282). Note that either > patch 1 or patch 2 in isolation will cause bootstrap failure; so even though > you normally like to do gnulib submodule updates in isolation, I'm wondering if > this is a case where the two patches should be squashed together. This sounds like a good reason indeed to include the gnulib update with the dependent change. FWIW, I'm less averse to including a gnulib submodule update with other changes, now that I've become more used to dealing with submodules. > [2/4] build: reflect gnulib changes to tempname > Since gnulib added mkostemps support to gen_tempname, we need to reflect the > extra parameter in gen_tempname_len. I think the .diff format makes it easier > to see how we intentionally differ from glibc, rather than writing a flat-out > replacement file (it's also more robust at catching subsequent gnulib changes > to the same file, to let us know to resynchronize). Yes, I was thinking the same thing. This would be easier to review if there were two deltas: - the no-semantic-change, convert-to-.diff one - the add-argument semantics-changing one but it's not that big a deal. Yeah, it's a shame about glibc using the incorrect type for the new parameter. > [3/4] tests: enhance mktemp test > Add more tests of existing behavior, to ensure I didn't break it, and to make > it obvious what I'm changing. I could rebase the series to put this first, > alongside my pending mktemp doc patch, especially if your review of my doc > patch and the accompanying email turn up any changes we want to existing > behavior. Those new tests look fine. > [4/4] mktemp: add suffix handling > The real fun. Hopefully I added enough tests to cover everything new that > results from the new code. Also, is the rearranged output in usage() okay, and > should I split that into a separate patch? I'll probably run out of time here, but will start commenting. AUTHORS: Glad you added your name. > In rereading this before hitting the send button, I see an out-of-bounds > reference in patch 4. I'll know if you reviewed this if you spot the same > bug ;) Also, I guess I should also amend my series to mention this bug report > in the commit message: >> > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316 Better to stop now. Will resume here in the morning and then revisit your doc-change message. |
|
|
Re: temp file suffixes: mktemp DWIMEric Blake wrote:
+ if (suffix) + { + size_t len = strlen (template); + if (template[len - 1] != 'X') If you pass '' the above is invalid + { + error (EXIT_FAILURE, 0, + _("with --suffix, template %s must end in X"), + quote (template)); + } + suffix_len = strlen (suffix); + dest_name = xcharalloc (len + suffix_len + 1); That's the first use of that function. Seems of marginal benefit. + memcpy (dest_name, template, len); + memcpy (dest_name + len, suffix, suffix_len + 1); + template = dest_name; + suffix = dest_name + len; + } Other than that it's a really nice patch. cheers, Pádraig. |
|
|
[PATCHv2 0/7] Re: temp file suffixes: mktemp DWIMJim Meyering <jim <at> meyering.net> writes:
> > patch 1 or patch 2 in isolation will cause bootstrap failure; so even though > > you normally like to do gnulib submodule updates in isolation, I'm wondering if > > this is a case where the two patches should be squashed together. > > This sounds like a good reason indeed to include the gnulib update > with the dependent change. Squashed together in the rebase. > This would be easier to review if there were two deltas: > - the no-semantic-change, convert-to-.diff one > - the add-argument semantics-changing one > but it's not that big a deal. Done in the rebase. > Yeah, it's a shame about glibc using the incorrect type > for the new parameter. Not glibc's fault. Uli was stuck copying prior art for portability - both BSD and Solaris previously implemented mkstemps with the wrong type (although I suppose that if glibc 2.11 hadn't been released in the same week as the glibc invention of mkostemps was implemented, we could have tried to ask for at least that interface to be right). > > Better to stop now. > Will resume here in the morning and then revisit your doc-change message. Here's the current state of my series, including the previously-reviewed stdout patch, and the doc-change. Eric Blake (7): [1/7] mktemp: don't leave file behind on write failure Already reviewed. [2/7] doc: document mktemp Doc patch of existing behavior. Slightly improved since last posting. [3/7] tests: enhance mktemp test More thorough testing of existing behavior. Already reviewed as 3/4 in previous round. [4/7] mktemp: rearrange --help output Tweak --help output to look nicer. Split out from 4/4 in previous round. [5/7] build: override gnulib tempname via diff I slightly tweaked the files when creating the diffs, so that the interdiff to the next patch is not as hairy, but there should be no semantic difference. For an example of the changes, tempname.c.diff no longer exludes a 20-line #if _LIBC hunk even though we had previously dropped it from tempname.c; and even though we removed TABs from tempname.c, tempname.c.diff uses TABs in order to be a closer match to upstream and its different whitespace preference). Split out from 2/4 of previous round. [6/7] build: reflect gnulib changes to tempname I guess I'd better push my gnulib mkostemps series soon, so that you can proceed to bootstrap and compile, rather than just inspect, this patch (the gnulib commit id is still TBD, but as long as your gnulib repository contains my mkostemps series, you should be able to figure out how to make it work). gmane will probably botch the long lines, and diffs of *.diff are a little bit tricky to read, but it does do a better job of showing what had to change as a result of the gnulib update. Merge of 1/4 and 2/4 of previous round. [7/7] mktemp: add suffix handling Fix out-of-bounds dereference (Pádraig won my challenge to spot it during his review). 4/4 in previous round. I'll post a link once I've actually pushed these to my personal repository on repo.or.cz, to make reviewing a bit easier (especially given the long lines). From 8d33fb00227d70d55b44dbfbadd69bfcf15aa614 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 14:02:20 -0700 Subject: [PATCH 1/7] mktemp: don't leave file behind on write failure * src/mktemp.c (main): Remove just-created file if stdout had problems. * bootstrap.conf (gnulib_modules): Add remove. * tests/misc/close-stdout: Test it. * NEWS: Document it. --- NEWS | 4 ++++ bootstrap.conf | 1 + src/mktemp.c | 13 ++++++++++++- tests/misc/close-stdout | 6 ++++++ 4 files changed, 23 insertions(+), 1 deletions(-) diff --git a/NEWS b/NEWS index 03ed83f..711eb74 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,10 @@ GNU coreutils NEWS -*- outline -*- This also affected sum, sha1sum, sha224sum, sha384sum and sha512sum. [the bug dates back to the initial implementation] + mktemp no longer leaves a temporary file behind if it was unable to + output the name of the file to stdout. + [the bug dates back to the initial implementation] + nice -n -1 PROGRAM now runs PROGRAM even when its internal setpriority call fails with errno == EACCES. [the bug dates back to the initial implementation] diff --git a/bootstrap.conf b/bootstrap.conf index 7960546..b3a82e0 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -180,6 +180,7 @@ gnulib_modules=" readutmp realloc regex + remove rename rmdir root-dev-ino diff --git a/src/mktemp.c b/src/mktemp.c index 808efa9..2b6e4b4 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -23,6 +23,7 @@ #include "system.h" +#include "close-stream.h" #include "error.h" #include "filenamecat.h" #include "quote.h" @@ -277,7 +278,17 @@ main (int argc, char **argv) } if (status == EXIT_SUCCESS) - puts (dest_name); + { + puts (dest_name); + /* If we created a file, but then failed to output the file + name, we should clean up the mess before failing. */ + if (!dry_run && close_stream (stdout)) + { + int saved_errno = errno; + remove (dest_name); + error (EXIT_FAILURE, saved_errno, _("write error")); + } + } #ifdef lint free (dest_name); diff --git a/tests/misc/close-stdout b/tests/misc/close-stdout index fec1add..852c3c8 100755 --- a/tests/misc/close-stdout +++ b/tests/misc/close-stdout @@ -50,12 +50,18 @@ if "$p/src/test" -w /dev/stdout >/dev/null && "$p/src/test" ! -w /dev/stdout >&-; then "$p/src/printf" 'foo' >&- 2>/dev/null && fail=1 cp --verbose a b >&- 2>/dev/null && fail=1 + rm -Rf tmpfile-?????? || fail=1 + mktemp tmpfile-XXXXXX >&- 2>/dev/null && fail=1 + test -e tmpfile-?????? && fail=1 fi # Likewise for /dev/full, if /dev/full works. if test -w /dev/full && test -c /dev/full; then "$p/src/printf" 'foo' >/dev/full 2>/dev/null && fail=1 cp --verbose a b >/dev/full 2>/dev/null && fail=1 + rm -Rf tmpdir-?????? || fail=1 + mktemp -d tmpdir-XXXXXX >/dev/full 2>/dev/null && fail=1 + test -e tmpdir-?????? && fail=1 fi Exit $fail -- 1.6.4.2 From 5958e44b9eeb2656d2ab47a4952d527d22529e1b Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 10:47:42 -0700 Subject: [PATCH 2/7] doc: document mktemp * doc/coreutils.texi (mktemp invocation): New node. * TODO: Delete completed task. --- TODO | 1 - doc/coreutils.texi | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 158 insertions(+), 3 deletions(-) diff --git a/TODO b/TODO index 9128ddf..96d5095 100644 --- a/TODO +++ b/TODO @@ -12,7 +12,6 @@ Modify chmod so that it does not change an inode's st_ctime Discussed more recently on <http://bugs.debian.org/497514>. document the following in coreutils.texi: - mktemp [ pinky diff --git a/doc/coreutils.texi b/doc/coreutils.texi index ec5bcfb..7c4f680 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -31,7 +31,6 @@ @c FIXME: the following need documentation @c * [: (coreutils)[ invocation. File/string tests. @c * pinky: (coreutils)pinky invocation. FIXME. -@c * mktemp: (coreutils)mktemp invocation. FIXME. @dircategory Individual utilities @direntry @@ -80,6 +79,7 @@ * mkdir: (coreutils)mkdir invocation. Create directories. * mkfifo: (coreutils)mkfifo invocation. Create FIFOs (named pipes). * mknod: (coreutils)mknod invocation. Create special files. +* mktemp: (coreutils)mktemp invocation. Create temporary files. * mv: (coreutils)mv invocation. Rename files. * nice: (coreutils)nice invocation. Modify niceness. * nl: (coreutils)nl invocation. Number lines and write files. @@ -193,7 +193,7 @@ Top * Printing text:: echo printf yes * Conditions:: false true test expr * Redirection:: tee -* File name manipulation:: dirname basename pathchk +* File name manipulation:: dirname basename pathchk mktemp * Working context:: pwd stty printenv tty * User information:: id logname whoami groups users who * System context:: date arch uname hostname hostid uptime @@ -378,6 +378,7 @@ Top * basename invocation:: Strip directory and suffix from a file name * dirname invocation:: Strip non-directory suffix from a file name * pathchk invocation:: Check file name validity and portability +* mktemp invocation:: Create temporary file or directory Working context @@ -11876,6 +11877,7 @@ File name manipulation * basename invocation:: Strip directory and suffix from a file name. * dirname invocation:: Strip non-directory suffix from a file name. * pathchk invocation:: Check file name validity and portability. +* mktemp invocation:: Create temporary file or directory. @end menu @@ -12050,6 +12052,160 @@ pathchk invocation 1 otherwise. @end display +@node mktemp invocation +@section @command{mktemp}: Create temporary file or directory + +@pindex mktemp +@cindex file names, creating temporary +@cindex directory, creating temporary +@cindex temporary files and directories + +@command{mktemp} manages the creation of temporary files and +directories. Synopsis: + +@example +mktemp [@var{option}]@dots{} [@var{template}] +@end example + +Safely create a temporary file or directory based on @var{template}, +and print its name. If given, @var{template} must end in at least +three consecutive @samp{X}. If omitted, the template +@samp{tmp.XXXXXXXXXX} is used, and option @option{--tmpdir} is +implied. The trailing @samp{X} in the @var{template} will be replaced +by alpha-numeric characters; thus, on a case-sensitive file system, +and with a @var{template} ending in @var{n} instances of @samp{X}, +there are @samp{62**@var{n}} potential file names. + +Older scripts used to create temporary files by simply joining the +name of the program with the process id (@samp{$$}) as a suffix. +However, that naming scheme is easily predictable, and suffers from a +race condition where the attacker can create an appropriately named +symbolic link, such that when the script then opens a handle to what +it thought was an unused file, it is instead modifying an existing +file. Using the same scheme to create a directory is slightly safer, +since the @command{mkdir} will fail if the target already exists, but +it is still inferior because it allows for denial of service attacks. +Therefore, modern scripts should use the @command{mktemp} command to +guarantee that the generated name will be unpredictable, and that +knowledge of the temporary file name implies that the file was created +by the current script and cannot be modified by other users. + +When creating a file, the resulting file has read and write +permissions for the current user, but no permissions for the group or +others; these permissions are reduced if the current umask is more +restrictive. + +Here are some examples (although note that if you repeat them, you +will most likely get different file names): + +@itemize @bullet + +@item +Create a temporary file in the current directory. +@example +$ mktemp file.XXXX +file.H47c +@end example + +@item +Create a secure fifo relative to the user's choice of @env{TMPDIR}, +but falling back to the current directory rather than @file{/tmp}. +Note that @command{mktemp} does not create fifos, but can create a +secure directory in which the fifo can live. Exit the shell if the +directory or fifo could not be created. +@example +$ dir=$(mktemp -p "$@{TMPDIR:-.@}" -d dir-XXXX) || exit 1 +$ fifo=$dir/fifo +$ mkfifo "$fifo" || @{ rmdir $dir; exit 1; @} +@end example + +@item +Create and use a temporary file if possible, but ignore failure. The +file will reside in the directory named by @env{TMPDIR}, if specified, +or else in @file{/tmp}. +@example +$ file=$(mktemp -q) && @{ +> # Safe to use $file only within this block +> echo ... > $file +> rm $file +> @} +@end example + +@item +Act as a semi-random character generator (it is not fully random, +since it is impacted by the contents of the current directory). To +avoid security holes, do not use the resulting names to create a file. +@example +$ mktemp -u XXX +Gb9 +$ mktemp -u XXX +nzC +@end example + +@end itemize + +The program accepts the following options. Also see @ref{Common options}. + +@table @samp + +@item -d +@itemx --directory +@opindex -d +@opindex --directory +Create a directory rather than a file. The directory will have read, +write, and search permissions for the current user, but no permissions +for the group or others; these permissions are reduced if the current +umask is more restrictive. + +@item -q +@itemx --quiet +@opindex -q +@opindex --quiet +Suppress diagnostics about failure to create a file or directory. The +exit status will still reflect whether a file was created. + +@item -u +@itemx --dry-run +@opindex -u +@opindex --dry-run +Generate a temporary name that does not name an existing file, without +changing the file system contents. Using the output of this command +to create a new file is inherently unsafe, as there is a window of +time between generating the name and using it where another process +can create an object by the same name. + +@item -p @var{dir} +@itemx --tempdir[=@var{dir}] +@opindex -p +@opindex --tempdir +Treat @var{template} relative to the directory @var{dir}. If +@var{dir} is not specified (only possible with the long option +@option{--tempdir}) or is the empty string, use the value of +@env{TMPDIR} if available, otherwise use @samp{/tmp}. If this is +specified, @var{template} must not be absolute. However, +@var{template} can still contain slashes, although intermediate +directories must already exist. + +@item -t +@opindex -t +Treat @var{template} as a single file relative to the value of +@env{TMPDIR} if available, or to the directory specified by +@option{-p}, otherwise to @samp{/tmp}. @var{template} must not +contain slashes. This option is deprecated; the use of @option{-p} +without @option{-t} offers better defaults (by favoring the command +line over @env{TMPDIR}) and more flexibility (by allowing intermediate +directories). + +@end table + +@cindex exit status of @command{mktemp} +Exit status: + +@display +0 if the file was created, +1 otherwise. +@end display + @node Working context @chapter Working context -- 1.6.4.2 From 34b5ced030591e2f2a9e22f1e3cfb1648bfa8939 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 09:01:49 -0700 Subject: [PATCH 3/7] tests: enhance mktemp test * tests/misc/mktemp: Add more coverage. --- tests/misc/mktemp | 28 +++++++++++++++++++++++++++- 1 files changed, 27 insertions(+), 1 deletions(-) diff --git a/tests/misc/mktemp b/tests/misc/mktemp index dde1532..e620311 100755 --- a/tests/misc/mktemp +++ b/tests/misc/mktemp @@ -62,6 +62,7 @@ my @Tests = ['too-few-x', 'foo.XX', {ERR=>"$prog: too few X's in template `foo.XX'\n"}, {EXIT => 1} ], + ['too-few-xq', '-q foo.XX', {EXIT => 1} ], ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, @@ -69,6 +70,12 @@ my @Tests = check_tmp $f, 'F'; }} ], + ['2f', '-- -XXXX', {OUT => "-ZZZZ\n"}, + {OUT_SUBST => 's,-....$,-ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }} + ], + # Create a temporary directory. ['1d', '-d f.XXXX', {OUT => "f.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, @@ -83,7 +90,18 @@ my @Tests = check_tmp $f, 'D'; }} ], - ['invalid-tmpl', '-t a/bXXXX', + # Test -u + ['uf', '-u f.XXXX', {OUT => "f.ZZZZ\n"}, + {OUT_SUBST => 's,\.....$,.ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created file"; }}], + ['ud', '-d --dry-run d.XXXX', {OUT => "d.ZZZZ\n"}, + {OUT_SUBST => 's,\.....$,.ZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created directory"; }}], + + # Test bad templates + ['invalid-tl', '-t a/bXXXX', {ERR=>"$prog: invalid template, `a/bXXXX', " . "contains directory separator\n"}, {EXIT => 1} ], @@ -91,6 +109,11 @@ my @Tests = {ERR=>"$prog: invalid template, `/bXXXX'; " . "with --tmpdir, it may not be absolute\n"}, {EXIT => 1} ], + # Suffix after X. + ['invalid-t3', 'aXXXXb', + {ERR=>"$prog: too few X's in template `aXXXXb'\n"}, {EXIT => 1} ], + + # Test template with subdirectory ['tmp-w-slash', '--tmpdir=. a/bXXXX', {PRE => sub {mkdir 'a',0755 or die "a: $!\n"}}, {OUT_SUBST => 's,b....$,bZZZZ,'}, @@ -104,6 +127,9 @@ my @Tests = {ERR_SUBST => "s,($bad_dir/)[^']+': .*,\$1...,"}, {ERR => "$prog: failed to create file via template `$bad_dir/...\n"}, {EXIT => 1}], + ['pipe-bad-tmpdir-u', '-u', {OUT => "$bad_dir/tmp.ZZZZZZZZZZ\n"}, + {ENV => "TMPDIR=$bad_dir"}, + {OUT_SUBST => 's,\..{10}$,.ZZZZZZZZZZ,'}], ); my $save_temps = $ENV{DEBUG}; -- 1.6.4.2 From 0173f8e4cb699abba758cebed8239b01b22db300 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 16:09:30 -0700 Subject: [PATCH 4/7] mktemp: rearrange --help output * src/mktemp.c (usage): Align indentation and sort by long options. Describe valid templates. Suggested by http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316. --- src/mktemp.c | 33 ++++++++++++++------------------- 1 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/mktemp.c b/src/mktemp.c index 2b6e4b4..704494b 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -65,33 +65,28 @@ usage (int status) printf (_("Usage: %s [OPTION]... [TEMPLATE]\n"), program_name); fputs (_("\ Create a temporary file or directory, safely, and print its name.\n\ +TEMPLATE must end in at least 3 consecutive X.\n\ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ "), stdout); fputs ("\n", stdout); fputs (_("\ - -d, --directory create a directory, not a file\n\ + -d, --directory create a directory, not a file\n\ + -u, --dry-run do not create anything; merely print a name (unsafe)\n\ + -q, --quiet suppress diagnostics about file/dir-creation failure\n\ "), stdout); fputs (_("\ - -q, --quiet suppress diagnostics about file/dir-creation failure\n\ -"), stdout); - fputs (_("\ - -u, --dry-run do not create anything; merely print a name (unsafe)\n\ -"), stdout); - fputs (_("\ - --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is\n\ - not specified, use $TMPDIR if set, else /tmp.\n\ - With this option, TEMPLATE must not be an absolute name.\n\ - Unlike with -t, TEMPLATE may contain slashes, but even\n\ - here, mktemp still creates only the final component.\n\ + --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is not\n\ + specified, use $TMPDIR if set, else /tmp. With\n\ + this option, TEMPLATE must not be an absolute name.\n\ + Unlike with -t, TEMPLATE may contain slashes, but\n\ + mktemp creates only the final component.\n\ "), stdout); fputs ("\n", stdout); fputs (_("\ - -p DIR use DIR as a prefix; implies -t [deprecated]\n\ -"), stdout); - fputs (_("\ - -t interpret TEMPLATE as a single file name component,\n\ - relative to a directory: $TMPDIR, if set; else the\n\ - directory specified via -p; else /tmp [deprecated]\n\ + -p DIR use DIR as a prefix; implies -t [deprecated]\n\ + -t interpret TEMPLATE as a single file name component,\n\ + relative to a directory: $TMPDIR, if set; else the\n\ + directory specified via -p; else /tmp [deprecated]\n\ "), stdout); fputs ("\n", stdout); fputs (HELP_OPTION_DESCRIPTION, stdout); @@ -178,7 +173,7 @@ main (int argc, char **argv) case_GETOPT_HELP_CHAR; - case 'V': + case 'V': /* Undocumented alias. FIXME: remove in 2011. */ case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); default: usage (EXIT_FAILURE); -- 1.6.4.2 From 9f0deea68fa9e9ef8557ae494b39529885e8bbe3 Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 08:47:24 -0700 Subject: [PATCH 5/7] build: override gnulib tempname via diff Diffs are more robust than wholesale replacement, because bootstrap will inform us of any incompatible changes made in upstream gnulib. * gl/lib/tempname.h: Change... * gl/lib/tempname.h.diff: ...to diff. * gl/lib/tempname.c: Change... * gl/lib/tempname.c.diff: ...to diff. --- gl/lib/tempname.c | 294 ------------------------------------------------ gl/lib/tempname.c.diff | 192 +++++++++++++++++++++++++++++++ gl/lib/tempname.h | 42 ------- gl/lib/tempname.h.diff | 12 ++ 4 files changed, 204 insertions(+), 336 deletions(-) delete mode 100644 gl/lib/tempname.c create mode 100644 gl/lib/tempname.c.diff delete mode 100644 gl/lib/tempname.h create mode 100644 gl/lib/tempname.h.diff diff --git a/gl/lib/tempname.c b/gl/lib/tempname.c deleted file mode 100644 index 84679bc..0000000 --- a/gl/lib/tempname.c +++ /dev/null @@ -1,294 +0,0 @@ -/* tempname.c - generate the name of a temporary file. - - Copyright (C) 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, - 2000, 2001, 2002, 2003, 2005, 2006, 2007, 2009 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 <http://www.gnu.org/licenses/>. */ - -/* Extracted from glibc sysdeps/posix/tempname.c. See also tmpdir.c. */ - -#if !_LIBC -# include <config.h> -# include "tempname.h" -# include "randint.h" -#endif - -#include <sys/types.h> -#include <assert.h> - -#include <errno.h> -#ifndef __set_errno -# define __set_errno(Val) errno = (Val) -#endif - -#include <stdio.h> -#ifndef P_tmpdir -# define P_tmpdir "/tmp" -#endif -#ifndef TMP_MAX -# define TMP_MAX 238328 -#endif -#ifndef __GT_FILE -# define __GT_FILE 1 -# define __GT_DIR 2 -# define __GT_NOCREATE 3 -#endif - -#include <stdbool.h> -#include <stddef.h> -#include <stdlib.h> -#include <string.h> - -#include <fcntl.h> -#include <sys/time.h> -#include <stdint.h> -#include <unistd.h> - -#include <sys/stat.h> - -#if _LIBC -# define struct_stat64 struct stat64 -#else -# define struct_stat64 struct stat -# define __open open -# define __gen_tempname gen_tempname -# define __getpid getpid -# define __gettimeofday gettimeofday -# define __mkdir mkdir -# define __lxstat64(version, file, buf) lstat (file, buf) -# define __xstat64(version, file, buf) stat (file, buf) -#endif - -#if ! (HAVE___SECURE_GETENV || _LIBC) -# define __secure_getenv getenv -#endif - -#if _LIBC -/* Return nonzero if DIR is an existent directory. */ -static int -direxists (const char *dir) -{ - struct_stat64 buf; - return __xstat64 (_STAT_VER, dir, &buf) == 0 && S_ISDIR (buf.st_mode); -} - -/* Path search algorithm, for tmpnam, tmpfile, etc. If DIR is - non-null and exists, uses it; otherwise uses the first of $TMPDIR, - P_tmpdir, /tmp that exists. Copies into TMPL a template suitable - for use with mk[s]temp. Will fail (-1) if DIR is non-null and - doesn't exist, none of the searched dirs exists, or there's not - enough space in TMPL. */ -int -__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, - int try_tmpdir) -{ - const char *d; - size_t dlen, plen; - - if (!pfx || !pfx[0]) - { - pfx = "file"; - plen = 4; - } - else - { - plen = strlen (pfx); - if (plen > 5) - plen = 5; - } - - if (try_tmpdir) - { - d = __secure_getenv ("TMPDIR"); - if (d != NULL && direxists (d)) - dir = d; - else if (dir != NULL && direxists (dir)) - /* nothing */ ; - else - dir = NULL; - } - if (dir == NULL) - { - if (direxists (P_tmpdir)) - dir = P_tmpdir; - else if (strcmp (P_tmpdir, "/tmp") != 0 && direxists ("/tmp")) - dir = "/tmp"; - else - { - __set_errno (ENOENT); - return -1; - } - } - - dlen = strlen (dir); - while (dlen > 1 && dir[dlen - 1] == '/') - dlen--; /* remove trailing slashes */ - - /* check we have room for "${dir}/${pfx}XXXXXX\0" */ - if (tmpl_len < dlen + 1 + plen + 6 + 1) - { - __set_errno (EINVAL); - return -1; - } - - sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx); - return 0; -} -#endif /* _LIBC */ - -static inline bool -check_x_suffix (char const *s, size_t len) -{ - return strspn (s, "X") == len; -} - -/* These are the characters used in temporary file names. */ -static const char letters[] = -"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; - -/* Generate a temporary file name based on TMPL. TMPL must end in a - a sequence of at least X_SUFFIX_LEN "X"s. The name constructed - does not exist at the time of the call to __gen_tempname. TMPL is - overwritten with the result. - - KIND may be one of: - __GT_NOCREATE: simply verify that the name does not exist - at the time of the call. - __GT_FILE: create the file using open(O_CREAT|O_EXCL) - and return a read-write fd. The file is mode 0600. - __GT_DIR: create a directory, which will be mode 0700. - - We use a clever algorithm to get hard-to-predict names. */ -int -gen_tempname_len (char *tmpl, int flags, int kind, size_t x_suffix_len) -{ - size_t len; - char *XXXXXX; - unsigned int count; - int fd = -1; - int save_errno = errno; - struct_stat64 st; - struct randint_source *rand_src; - - /* A lower bound on the number of temporary files to attempt to - generate. The maximum total number of temporary file names that - can exist for a given template is 62**6. It should never be - necessary to try all these combinations. Instead if a reasonable - number of names is tried (we define reasonable as 62**3) fail to - give the system administrator the chance to remove the problems. */ -#define ATTEMPTS_MIN (62 * 62 * 62) - - /* The number of times to attempt to generate a temporary file. To - conform to POSIX, this must be no smaller than TMP_MAX. */ -#if ATTEMPTS_MIN < TMP_MAX - unsigned int attempts = TMP_MAX; -#else - unsigned int attempts = ATTEMPTS_MIN; -#endif - - len = strlen (tmpl); - if (len < x_suffix_len || ! check_x_suffix (&tmpl[len - x_suffix_len], - x_suffix_len)) - { - __set_errno (EINVAL); - return -1; - } - - rand_src = randint_all_new (NULL, 8); - if (! rand_src) - return -1; - - /* This is where the Xs start. */ - XXXXXX = &tmpl[len - x_suffix_len]; - - for (count = 0; count < attempts; ++count) - { - size_t i; - - for (i = 0; i < x_suffix_len; i++) - { - XXXXXX[i] = letters[randint_genmax (rand_src, sizeof letters - 2)]; - } - - switch (kind) - { - case __GT_FILE: - fd = __open (tmpl, - (flags & ~0777) | O_RDWR | O_CREAT | O_EXCL, - S_IRUSR | S_IWUSR); - break; - - case __GT_DIR: - fd = __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR); - break; - - case __GT_NOCREATE: - /* This case is backward from the other three. This function - succeeds if __xstat fails because the name does not exist. - Note the continue to bypass the common logic at the bottom - of the loop. */ - if (__lxstat64 (_STAT_VER, tmpl, &st) < 0) - { - if (errno == ENOENT) - { - __set_errno (save_errno); - fd = 0; - goto done; - } - else - { - /* Give up now. */ - fd = -1; - goto done; - } - } - continue; - - default: - assert (! "invalid KIND in __gen_tempname"); - } - - if (fd >= 0) - { - __set_errno (save_errno); - goto done; - } - else if (errno != EEXIST) - { - fd = -1; - goto done; - } - } - - randint_all_free (rand_src); - - /* We got out of the loop because we ran out of combinations to try. */ - __set_errno (EEXIST); - return -1; - - done: - { - int saved_errno = errno; - randint_all_free (rand_src); - __set_errno (saved_errno); - } - return fd; -} - -int -__gen_tempname (char *tmpl, int flags, int kind) -{ - return gen_tempname_len (tmpl, flags, kind, 6); -} diff --git a/gl/lib/tempname.c.diff b/gl/lib/tempname.c.diff new file mode 100644 index 0000000..2b6c58f --- /dev/null +++ b/gl/lib/tempname.c.diff @@ -0,0 +1,192 @@ +diff --git c/lib/tempname.c i/lib/tempname.c +index 4102134..a03346e 100644 +--- c/lib/tempname.c ++++ i/lib/tempname.c +@@ -22,6 +22,7 @@ + #if !_LIBC + # include <config.h> + # include "tempname.h" ++# include "randint.h" + #endif + + #include <sys/types.h> +@@ -45,6 +46,7 @@ + # define __GT_NOCREATE 3 + #endif + ++#include <stdbool.h> + #include <stddef.h> + #include <stdlib.h> + #include <string.h> +@@ -174,14 +176,20 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, + } + #endif /* _LIBC */ + ++static inline bool ++check_x_suffix (char const *s, size_t len) ++{ ++ return strspn (s, "X") == len; ++} ++ + /* These are the characters used in temporary file names. */ + static const char letters[] = + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + +-/* Generate a temporary file name based on TMPL. TMPL must match the +- rules for mk[s]temp (i.e. end in "XXXXXX"). The name constructed +- does not exist at the time of the call to __gen_tempname. TMPL is +- overwritten with the result. ++/* Generate a temporary file name based on TMPL. TMPL must end in a ++ a sequence of at least X_SUFFIX_LEN "X"s. ++ The name constructed does not exist at the time of the call to ++ this function. TMPL is overwritten with the result. + + KIND may be one of: + __GT_NOCREATE: simply verify that the name does not exist +@@ -192,23 +200,23 @@ static const char letters[] = + + We use a clever algorithm to get hard-to-predict names. */ + int +-__gen_tempname (char *tmpl, int flags, int kind) ++gen_tempname_len (char *tmpl, int flags, int kind, size_t x_suffix_len) + { +- int len; ++ size_t len; + char *XXXXXX; +- static uint64_t value; +- uint64_t random_time_bits; + unsigned int count; + int fd = -1; + int save_errno = errno; + struct_stat64 st; ++ struct randint_source *rand_src; + + /* A lower bound on the number of temporary files to attempt to + generate. The maximum total number of temporary file names that + can exist for a given template is 62**6. It should never be + necessary to try all these combinations. Instead if a reasonable + number of names is tried (we define reasonable as 62**3) fail to +- give the system administrator the chance to remove the problems. */ ++ give the system administrator the chance to remove the problems. ++ This value requires that X_SUFFIX_LEN be at least 3. */ + #define ATTEMPTS_MIN (62 * 62 * 62) + + /* The number of times to attempt to generate a temporary file. To +@@ -220,43 +228,27 @@ __gen_tempname (char *tmpl, int flags, int kind) + #endif + + len = strlen (tmpl); +- if (len < 6 || strcmp (&tmpl[len - 6], "XXXXXX")) ++ if (len < x_suffix_len || ! check_x_suffix (&tmpl[len - x_suffix_len], ++ x_suffix_len)) + { + __set_errno (EINVAL); + return -1; + } + + /* This is where the Xs start. */ +- XXXXXX = &tmpl[len - 6]; ++ XXXXXX = &tmpl[len - x_suffix_len]; + + /* Get some more or less random data. */ +-#ifdef RANDOM_BITS +- RANDOM_BITS (random_time_bits); +-#else +- { +- struct timeval tv; +- __gettimeofday (&tv, NULL); +- random_time_bits = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec; +- } +-#endif +- value += random_time_bits ^ __getpid (); ++ rand_src = randint_all_new (NULL, 8); ++ if (! rand_src) ++ return -1; + +- for (count = 0; count < attempts; value += 7777, ++count) ++ for (count = 0; count < attempts; ++count) + { +- uint64_t v = value; +- +- /* Fill in the random bits. */ +- XXXXXX[0] = letters[v % 62]; +- v /= 62; +- XXXXXX[1] = letters[v % 62]; +- v /= 62; +- XXXXXX[2] = letters[v % 62]; +- v /= 62; +- XXXXXX[3] = letters[v % 62]; +- v /= 62; +- XXXXXX[4] = letters[v % 62]; +- v /= 62; +- XXXXXX[5] = letters[v % 62]; ++ size_t i; ++ ++ for (i = 0; i < x_suffix_len; i++) ++ XXXXXX[i] = letters[randint_genmax (rand_src, sizeof letters - 2)]; + + switch (kind) + { +@@ -271,7 +263,7 @@ __gen_tempname (char *tmpl, int flags, int kind) + break; + + case __GT_NOCREATE: +- /* This case is backward from the other three. __gen_tempname ++ /* This case is backward from the other three. This function + succeeds if __xstat fails because the name does not exist. + Note the continue to bypass the common logic at the bottom + of the loop. */ +@@ -280,11 +272,15 @@ __gen_tempname (char *tmpl, int flags, int kind) + if (errno == ENOENT) + { + __set_errno (save_errno); +- return 0; ++ fd = 0; ++ goto done; + } + else +- /* Give up now. */ +- return -1; ++ { ++ /* Give up now. */ ++ fd = -1; ++ goto done; ++ } + } + continue; + +@@ -295,13 +291,32 @@ __gen_tempname (char *tmpl, int flags, int kind) + if (fd >= 0) + { + __set_errno (save_errno); +- return fd; ++ goto done; + } + else if (errno != EEXIST) +- return -1; ++ { ++ fd = -1; ++ goto done; ++ } + } + ++ randint_all_free (rand_src); ++ + /* We got out of the loop because we ran out of combinations to try. */ + __set_errno (EEXIST); + return -1; ++ ++ done: ++ { ++ int saved_errno = errno; ++ randint_all_free (rand_src); ++ __set_errno (saved_errno); ++ } ++ return fd; ++} ++ ++int ++__gen_tempname (char *tmpl, int flags, int kind) ++{ ++ return gen_tempname_len (tmpl, flags, kind, 6); + } diff --git a/gl/lib/tempname.h b/gl/lib/tempname.h deleted file mode 100644 index a942f07..0000000 --- a/gl/lib/tempname.h +++ /dev/null @@ -1,42 +0,0 @@ -/* Create a temporary file or directory. - - Copyright (C) 2006, 2007, 2009 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 <http://www.gnu.org/licenses/>. */ - -/* header written by Eric Blake */ - -/* In gnulib, always prefer large files. GT_FILE maps to - __GT_BIGFILE, not __GT_FILE, for a reason. */ -#define GT_FILE 1 -#define GT_DIR 2 -#define GT_NOCREATE 3 - -/* Generate a temporary file name based on TMPL. TMPL must match the - rules for mk[s]temp (i.e. end in "XXXXXX"). The name constructed - does not exist at the time of the call to gen_tempname. TMPL is - overwritten with the result. - - KIND may be one of: - GT_NOCREATE: simply verify that the name does not exist - at the time of the call. - GT_FILE: create a large file using open(O_CREAT|O_EXCL) - and return a read-write fd. The file is mode 0600. - GT_DIR: create a directory, which will be mode 0700. - - We use a clever algorithm to get hard-to-predict names. */ -#include <stddef.h> -extern int gen_tempname (char *tmpl, int flags, int kind); -extern int gen_tempname_len (char *tmpl, int flags, int kind, - size_t x_suffix_len); diff --git a/gl/lib/tempname.h.diff b/gl/lib/tempname.h.diff new file mode 100644 index 0000000..5ceff5e --- /dev/null +++ b/gl/lib/tempname.h.diff @@ -0,0 +1,12 @@ +diff --git c/lib/tempname.h i/lib/tempname.h +index edf7074..707edf4 100644 +--- c/lib/tempname.h ++++ i/lib/tempname.h +@@ -34,4 +34,7 @@ + GT_DIR: create a directory, which will be mode 0700. + + We use a clever algorithm to get hard-to-predict names. */ ++#include <stddef.h> + extern int gen_tempname (char *tmpl, int flags, int kind); ++extern int gen_tempname_len (char *tmpl, int flags, int kind, ++ size_t x_suffix_len); -- 1.6.4.2 From 999a22e8e2a1826a02ea9d8ad97bf63d0f5939ac Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Tue, 3 Nov 2009 08:51:31 -0700 Subject: [PATCH 6/7] build: reflect gnulib changes to tempname In glibc 2.11 and gnulib, gen_tempname added a parameter suffixlen (unfortunately, it is typed as int rather than size_t, for historical compatibility to a poor choice by BSD). * gnulib: Import latest changes. * gl/lib/tempname.h.diff: Accommodate new suffixlen parameter. * gl/lib/tempname.c.diff (check_x_suffix): Allow for X in suffix beyond x_suffix_len. (gen_tempname_len): Add suffixlen parameter. (__gen_tempname): Update caller. * src/mktemp.c (mkstemp_len, mkdtemp_len): Update callers. --- gl/lib/tempname.c.diff | 53 ++++++++++++++++++++++++----------------------- gl/lib/tempname.h.diff | 12 +++++----- gnulib | 2 +- src/mktemp.c | 6 +++- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/gl/lib/tempname.c.diff b/gl/lib/tempname.c.diff index 2b6c58f..8ffc506 100644 --- a/gl/lib/tempname.c.diff +++ b/gl/lib/tempname.c.diff @@ -1,5 +1,5 @@ diff --git c/lib/tempname.c i/lib/tempname.c -index 4102134..a03346e 100644 +index 2da5afe..562955a 100644 --- c/lib/tempname.c +++ i/lib/tempname.c @@ -22,6 +22,7 @@ @@ -10,45 +10,45 @@ index 4102134..a03346e 100644 #endif #include <sys/types.h> -@@ -45,6 +46,7 @@ - # define __GT_NOCREATE 3 +@@ -49,6 +50,7 @@ + # error report this to bug-gnulib@... #endif +#include <stdbool.h> #include <stddef.h> #include <stdlib.h> #include <string.h> -@@ -174,14 +176,20 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, +@@ -179,14 +181,21 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, } #endif /* _LIBC */ +static inline bool +check_x_suffix (char const *s, size_t len) +{ -+ return strspn (s, "X") == len; ++ return len <= strspn (s, "X"); +} + /* These are the characters used in temporary file names. */ static const char letters[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; --/* Generate a temporary file name based on TMPL. TMPL must match the -- rules for mk[s]temp (i.e. end in "XXXXXX"). The name constructed -- does not exist at the time of the call to __gen_tempname. TMPL is -- overwritten with the result. -+/* Generate a temporary file name based on TMPL. TMPL must end in a -+ a sequence of at least X_SUFFIX_LEN "X"s. -+ The name constructed does not exist at the time of the call to + /* Generate a temporary file name based on TMPL. TMPL must match the +- rules for mk[s]temp (i.e. end in "XXXXXX", possibly with a suffix). ++ rules for mk[s]temp (i.e. end in at least X_SUFFIX_LEN "X"s, ++ possibly with a suffix). + The name constructed does not exist at the time of the call to +- __gen_tempname. TMPL is overwritten with the result. + this function. TMPL is overwritten with the result. KIND may be one of: __GT_NOCREATE: simply verify that the name does not exist -@@ -192,23 +200,23 @@ static const char letters[] = +@@ -197,23 +206,24 @@ static const char letters[] = We use a clever algorithm to get hard-to-predict names. */ int --__gen_tempname (char *tmpl, int flags, int kind) -+gen_tempname_len (char *tmpl, int flags, int kind, size_t x_suffix_len) +-__gen_tempname (char *tmpl, int suffixlen, int flags, int kind) ++gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind, ++ size_t x_suffix_len) { - int len; + size_t len; @@ -72,21 +72,22 @@ index 4102134..a03346e 100644 #define ATTEMPTS_MIN (62 * 62 * 62) /* The number of times to attempt to generate a temporary file. To -@@ -220,43 +228,27 @@ __gen_tempname (char *tmpl, int flags, int kind) +@@ -225,43 +235,28 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) #endif len = strlen (tmpl); -- if (len < 6 || strcmp (&tmpl[len - 6], "XXXXXX")) -+ if (len < x_suffix_len || ! check_x_suffix (&tmpl[len - x_suffix_len], -+ x_suffix_len)) +- if (len < 6 + suffixlen || memcmp (&tmpl[len - 6 - suffixlen], "XXXXXX", 6)) ++ if (len < x_suffix_len + suffixlen ++ || ! check_x_suffix (&tmpl[len - x_suffix_len - suffixlen], ++ x_suffix_len)) { __set_errno (EINVAL); return -1; } /* This is where the Xs start. */ -- XXXXXX = &tmpl[len - 6]; -+ XXXXXX = &tmpl[len - x_suffix_len]; +- XXXXXX = &tmpl[len - 6 - suffixlen]; ++ XXXXXX = &tmpl[len - x_suffix_len - suffixlen]; /* Get some more or less random data. */ -#ifdef RANDOM_BITS @@ -127,7 +128,7 @@ index 4102134..a03346e 100644 switch (kind) { -@@ -271,7 +263,7 @@ __gen_tempname (char *tmpl, int flags, int kind) +@@ -276,7 +271,7 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) break; case __GT_NOCREATE: @@ -136,7 +137,7 @@ index 4102134..a03346e 100644 succeeds if __xstat fails because the name does not exist. Note the continue to bypass the common logic at the bottom of the loop. */ -@@ -280,11 +272,15 @@ __gen_tempname (char *tmpl, int flags, int kind) +@@ -285,11 +280,15 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) if (errno == ENOENT) { __set_errno (save_errno); @@ -155,7 +156,7 @@ index 4102134..a03346e 100644 } continue; -@@ -295,13 +291,32 @@ __gen_tempname (char *tmpl, int flags, int kind) +@@ -301,13 +300,32 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind) if (fd >= 0) { __set_errno (save_errno); @@ -186,7 +187,7 @@ index 4102134..a03346e 100644 +} + +int -+__gen_tempname (char *tmpl, int flags, int kind) ++__gen_tempname (char *tmpl, int suffixlen, int flags, int kind) +{ -+ return gen_tempname_len (tmpl, flags, kind, 6); ++ return gen_tempname_len (tmpl, suffixlen, flags, kind, 6); } diff --git a/gl/lib/tempname.h.diff b/gl/lib/tempname.h.diff index 5ceff5e..6707445 100644 --- a/gl/lib/tempname.h.diff +++ b/gl/lib/tempname.h.diff @@ -1,12 +1,12 @@ diff --git c/lib/tempname.h i/lib/tempname.h -index edf7074..707edf4 100644 +index cd69e7d..9757db2 100644 --- c/lib/tempname.h +++ i/lib/tempname.h -@@ -34,4 +34,7 @@ - GT_DIR: create a directory, which will be mode 0700. +@@ -46,5 +46,7 @@ We use a clever algorithm to get hard-to-predict names. */ -+#include <stddef.h> - extern int gen_tempname (char *tmpl, int flags, int kind); -+extern int gen_tempname_len (char *tmpl, int flags, int kind, + extern int gen_tempname (char *tmpl, int suffixlen, int flags, int kind); ++extern int gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind, + size_t x_suffix_len); + + #endif /* GL_TEMPNAME_H */ diff --git a/gnulib b/gnulib index 5ff8115..a820a49 160000 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 5ff811558adf7013f9fd9109fa794dd4e5ee8c91 +Subproject commit a820a49944558a7f956f1c6ed476fb1e90fff313 diff --git a/src/mktemp.c b/src/mktemp.c index 704494b..2b86cb6 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -110,13 +110,15 @@ count_trailing_X_s (const char *s) static int mkstemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_FILE, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_FILE, + suff_len); } static int mkdtemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_DIR, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_DIR, + suff_len); } int -- 1.6.4.2 From 9b8faea4b195d6f742fd8dfd3a0595d385c2800b Mon Sep 17 00:00:00 2001 From: Eric Blake <ebb9@...> Date: Wed, 4 Nov 2009 11:13:39 -0700 Subject: [PATCH 7/7] mktemp: add suffix handling Now that mkstemps is supported, we might as well use it. * src/mktemp.c (TMPDIR_OPTION): New enum value. (longopts): Add new option. (usage): Document it. (count_trailing_X_s): Rename... (count_consecutive_X_s): ...to this, and add parameter. (mkstemp_len, mkdtemp_len): Add parameter. (main): Implement new option. (AUTHORS): Add myself. * AUTHORS (mktemp): Likewise. * tests/misc/mktemp: Test new option. * doc/coreutils.texi (mktemp invocation): Document it. * NEWS: Likewise. Fixes http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=548316. --- AUTHORS | 2 +- NEWS | 4 ++ doc/coreutils.texi | 27 ++++++++++++++-- src/mktemp.c | 86 ++++++++++++++++++++++++++++++++++++++++------------ tests/misc/mktemp | 72 +++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 162 insertions(+), 29 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7095db0..59a0dfa 100644 --- a/AUTHORS +++ b/AUTHORS @@ -46,7 +46,7 @@ md5sum: Ulrich Drepper, Scott Miller, David Madore mkdir: David MacKenzie mkfifo: David MacKenzie mknod: David MacKenzie -mktemp: Jim Meyering +mktemp: Jim Meyering, Eric Blake mv: Mike Parker, David MacKenzie, Jim Meyering nice: David MacKenzie nl: Scott Bartram, David MacKenzie diff --git a/NEWS b/NEWS index 711eb74..4d1b4f5 100644 --- a/NEWS +++ b/NEWS @@ -67,6 +67,10 @@ GNU coreutils NEWS -*- outline -*- md5sum --check now also accepts openssl-style checksums. So do sha1sum, sha224sum, sha384sum and sha512sum. + mktemp now accepts the option --suffix to provide a known suffix + after the substitution in the template. Additionally, uses such as + "mktemp fileXXXXXX.txt" are able to infer an appropriate --suffix. + touch now accepts the option --no-dereference (-h), as a means to change symlink timestamps on platforms with enough support. diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 7c4f680..aba5a17 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -12068,12 +12068,12 @@ mktemp invocation @end example Safely create a temporary file or directory based on @var{template}, -and print its name. If given, @var{template} must end in at least -three consecutive @samp{X}. If omitted, the template +and print its name. If given, @var{template} must include at least +three consecutive @samp{X} in the last component. If omitted, the template @samp{tmp.XXXXXXXXXX} is used, and option @option{--tmpdir} is -implied. The trailing @samp{X} in the @var{template} will be replaced +implied. The final run of @samp{X} in the @var{template} will be replaced by alpha-numeric characters; thus, on a case-sensitive file system, -and with a @var{template} ending in @var{n} instances of @samp{X}, +and with a @var{template} including a run of @var{n} instances of @samp{X}, there are @samp{62**@var{n}} potential file names. Older scripts used to create temporary files by simply joining the @@ -12108,6 +12108,15 @@ mktemp invocation @end example @item +Create a temporary file with a known suffix. +@example +$ mktemp --suffix=.txt file-XXXX +file-H08W.txt +$ mktemp file-XXXX-XXXX.txt +file-XXXX-eI9L.txt +@end example + +@item Create a secure fifo relative to the user's choice of @env{TMPDIR}, but falling back to the current directory rather than @file{/tmp}. Note that @command{mktemp} does not create fifos, but can create a @@ -12186,6 +12195,16 @@ mktemp invocation @var{template} can still contain slashes, although intermediate directories must already exist. +@item --suffix=@var{suffix} +@opindex --suffix +Append @var{suffix} to the @var{template}. @var{suffix} must not +contain slash. If @option{--suffix} is specified, @var{template} must +end in @samp{X}; if it is not specified, then an appropriate +@option{--suffix} is inferred by finding the last @samp{X} in +@var{template}. This option exists for use with the default +@var{template} and for the creation of a @var{suffix} that starts with +@samp{X}. + @item -t @opindex -t Treat @var{template} as a single file relative to the value of diff --git a/src/mktemp.c b/src/mktemp.c index 2b86cb6..3278bb2 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -14,7 +14,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ -/* Written by Jim Meyering. */ +/* Written by Jim Meyering and Eric Blake. */ #include <config.h> #include <stdio.h> @@ -32,7 +32,9 @@ /* The official name of this program (e.g., no `g' prefix). */ #define PROGRAM_NAME "mktemp" -#define AUTHORS proper_name ("Jim Meyering") +#define AUTHORS \ + proper_name ("Jim Meyering"), \ + proper_name ("Eric Blake") static const char *default_template = "tmp.XXXXXXXXXX"; @@ -40,7 +42,8 @@ static const char *default_template = "tmp.XXXXXXXXXX"; non-character as a pseudo short option, starting with CHAR_MAX + 1. */ enum { - TMPDIR_OPTION = CHAR_MAX + 1 + SUFFIX_OPTION = CHAR_MAX + 1, + TMPDIR_OPTION }; static struct option const longopts[] = @@ -48,6 +51,7 @@ static struct option const longopts[] = {"directory", no_argument, NULL, 'd'}, {"quiet", no_argument, NULL, 'q'}, {"dry-run", no_argument, NULL, 'u'}, + {"suffix", required_argument, NULL, SUFFIX_OPTION}, {"tmpdir", optional_argument, NULL, TMPDIR_OPTION}, {GETOPT_HELP_OPTION_DECL}, {GETOPT_VERSION_OPTION_DECL}, @@ -65,7 +69,7 @@ usage (int status) printf (_("Usage: %s [OPTION]... [TEMPLATE]\n"), program_name); fputs (_("\ Create a temporary file or directory, safely, and print its name.\n\ -TEMPLATE must end in at least 3 consecutive X.\n\ +TEMPLATE must contain at least 3 consecutive X in last component.\n\ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ "), stdout); fputs ("\n", stdout); @@ -75,6 +79,10 @@ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ -q, --quiet suppress diagnostics about file/dir-creation failure\n\ "), stdout); fputs (_("\ + --suffix=SUFF append SUFF to TEMPLATE. SUFF must not contain slash.\n\ + This option is implied if TEMPLATE does not end in X.\n\ +"), stdout); + fputs (_("\ --tmpdir[=DIR] interpret TEMPLATE relative to DIR. If DIR is not\n\ specified, use $TMPDIR if set, else /tmp. With\n\ this option, TEMPLATE must not be an absolute name.\n\ @@ -98,9 +106,8 @@ If TEMPLATE is not specified, use tmp.XXXXXXXXXX.\n\ } static size_t -count_trailing_X_s (const char *s) +count_consecutive_X_s (const char *s, size_t len) { - size_t len = strlen (s); size_t n = 0; for ( ; len && s[len-1] == 'X'; len--) ++n; @@ -108,17 +115,17 @@ count_trailing_X_s (const char *s) } static int -mkstemp_len (char *tmpl, size_t suff_len, bool dry_run) +mkstemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_FILE, - suff_len); + return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_FILE, + x_len); } static int -mkdtemp_len (char *tmpl, size_t suff_len, bool dry_run) +mkdtemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_DIR, - suff_len); + return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_DIR, + x_len); } int @@ -130,12 +137,14 @@ main (int argc, char **argv) int c; unsigned int n_args; char *template; + char *suffix = NULL; bool use_dest_dir = false; bool deprecated_t_option = false; bool create_directory = false; bool dry_run = false; int status = EXIT_SUCCESS; size_t x_count; + size_t suffix_len; char *dest_name; initialize_main (&argc, &argv); @@ -173,6 +182,10 @@ main (int argc, char **argv) dest_dir_arg = optarg; break; + case SUFFIX_OPTION: + suffix = optarg; + break; + case_GETOPT_HELP_CHAR; case 'V': /* Undocumented alias. FIXME: remove in 2011. */ @@ -208,7 +221,41 @@ main (int argc, char **argv) template = argv[optind]; } - x_count = count_trailing_X_s (template); + if (suffix) + { + size_t len = strlen (template); + if (!len || template[len - 1] != 'X') + { + error (EXIT_FAILURE, 0, + _("with --suffix, template %s must end in X"), + quote (template)); + } + suffix_len = strlen (suffix); + dest_name = xcharalloc (len + suffix_len + 1); + memcpy (dest_name, template, len); + memcpy (dest_name + len, suffix, suffix_len + 1); + template = dest_name; + suffix = dest_name + len; + } + else + { + template = xstrdup (template); + suffix = strrchr (template, 'X'); + if (!suffix) + suffix = strchr (template, '\0'); + else + suffix++; + suffix_len = strlen (suffix); + } + + /* At this point, template is malloc'd, and suffix points into template. */ + if (suffix_len && last_component (suffix) != suffix) + { + error (EXIT_FAILURE, 0, + _("invalid suffix %s, contains directory separator"), + quote (suffix)); + } + x_count = count_consecutive_X_s (template, suffix - template); if (x_count < 3) error (EXIT_FAILURE, 0, _("too few X's in template %s"), quote (template)); @@ -242,11 +289,10 @@ main (int argc, char **argv) quote (template)); } - template = file_name_concat (dest_dir, template, NULL); - } - else - { - template = xstrdup (template); + dest_name = file_name_concat (dest_dir, template, NULL); + free (template); + template = dest_name; + /* Note that suffix is now invalid. */ } /* Make a copy to be used in case of diagnostic, since failing @@ -255,7 +301,7 @@ main (int argc, char **argv) if (create_directory) { - int err = mkdtemp_len (dest_name, x_count, dry_run); + int err = mkdtemp_len (dest_name, suffix_len, x_count, dry_run); if (err != 0) { error (0, errno, _("failed to create directory via template %s"), @@ -265,7 +311,7 @@ main (int argc, char **argv) } else { - int fd = mkstemp_len (dest_name, x_count, dry_run); + int fd = mkstemp_len (dest_name, suffix_len, x_count, dry_run); if (fd < 0 || (!dry_run && close (fd) != 0)) { error (0, errno, _("failed to create file via template %s"), diff --git a/tests/misc/mktemp b/tests/misc/mktemp index e620311..7735f33 100755 --- a/tests/misc/mktemp +++ b/tests/misc/mktemp @@ -60,8 +60,8 @@ my @Tests = . "Try `$prog --help' for more information.\n"}, {EXIT => 1} ], ['too-many-q', '-q a b', {EXIT => 1} ], - ['too-few-x', 'foo.XX', - {ERR=>"$prog: too few X's in template `foo.XX'\n"}, {EXIT => 1} ], + ['too-few-x', 'foo.XX', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `foo.XX'\n"}], ['too-few-xq', '-q foo.XX', {EXIT => 1} ], ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, @@ -110,8 +110,72 @@ my @Tests = . "with --tmpdir, it may not be absolute\n"}, {EXIT => 1} ], # Suffix after X. - ['invalid-t3', 'aXXXXb', - {ERR=>"$prog: too few X's in template `aXXXXb'\n"}, {EXIT => 1} ], + ['suffix1f', 'aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + ['suffix1d', '-d aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'D'; }}], + ['suffix1u', '-u aXXXXb', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + -e $f and die "dry-run created file"; }}], + + ['suffix2f', 'aXXXXaaXXXXa', {OUT=>"aXXXXaaZZZZa\n"}, + {OUT_SUBST=>'s,a....a$,aZZZZa,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + ['suffix2d', '-d --suffix= aXXXXaaXXXX', {OUT=>"aXXXXaaZZZZ\n"}, + {OUT_SUBST=>'s,a....$,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'D'; }}], + + ['suffix3f', '--suffix=b aXXXX', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,a....b,aZZZZb,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix4f', '--suffix=X aXXXX', {OUT=>"aZZZZX\n"}, + {OUT_SUBST=>'s,^a....,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix5f', '--suffix /b aXXXX', {EXIT=>1}, + {ERR=>"$prog: invalid suffix `/b', contains directory separator\n"}], + + ['suffix6f', 'aXXXX/b', {EXIT=>1}, + {ERR=>"$prog: invalid suffix `/b', contains directory separator\n"}], + ['suffix6f-q', '-q aXXXX/b', {EXIT=>1}], + + ['suffix7f', '--suffix= aXXXXb', {EXIT=>1}, + {ERR=>"$prog: with --suffix, template `aXXXXb' must end in X\n"}], + ['suffix7f-q', '-q --suffix= aXXXXb', {EXIT=>1}], + ['suffix7d', '-d --suffix=aXXXXb ""', {EXIT=>1}, + {ERR=>"$prog: with --suffix, template `' must end in X\n"}], + + ['suffix8f', 'aXXXX --suffix=b', {OUT=>"aZZZZb\n"}, + {OUT_SUBST=>'s,^a....,aZZZZ,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + + ['suffix9f', 'aXXXX --suffix=b', {EXIT=>1}, + {ENV=>"POSIXLY_CORRECT=1"}, + {ERR=>"$prog: too many templates\n" + . "Try `$prog --help' for more information.\n"}], + + ['suffix10f', 'aXXb', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `aXXb'\n"}], + ['suffix10d', '-d --suffix=X aXX', {EXIT => 1}, + {ERR=>"$prog: too few X's in template `aXXX'\n"}], + + ['suffix11f', '--suffix=.txt', {OUT=>"./tmp.ZZZZZZZZZZ.txt\n"}, + {ENV=>"TMPDIR=."}, + {OUT_SUBST=>'s,\..{10}\.,.ZZZZZZZZZZ.,'}, + {POST => sub { my ($f) = @_; defined $f or return; chomp $f; + check_tmp $f, 'F'; }}], + # Test template with subdirectory ['tmp-w-slash', '--tmpdir=. a/bXXXX', -- 1.6.4.2 |
|
|
Re: [PATCHv2 0/7] Re: temp file suffixes: mktemp DWIM-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1 According to Eric Blake on 11/4/2009 5:13 PM: > I'll post a link once I've actually pushed these to my personal repository on > repo.or.cz, to make reviewing a bit easier (especially given the long lines). http://repo.or.cz/w/coreutils/ericb.git git pull git://repo.or.cz/coreutils/ericb.git master - -- Don't work too hard, make some time for fun as well! Eric Blake ebb9@... -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkryPRgACgkQ84KuGfSFAYBPcwCgwiRpyg1SGwleBx2MTEe6Ndcj ImEAoIiBs1NwGNqmQKMhlebkmiV1VZ/v =ywri -----END PGP SIGNATURE----- |
|
|
Re: [PATCHv2 0/7] Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> According to Eric Blake on 11/4/2009 5:13 PM: >> I'll post a link once I've actually pushed these to my personal repository on >> repo.or.cz, to make reviewing a bit easier (especially given the long lines). > > http://repo.or.cz/w/coreutils/ericb.git > git pull git://repo.or.cz/coreutils/ericb.git master Nice work! Modulo tiny comments, you're welcome to push that series once you've pushed the gnulib bits upon which it depends. As you know, with this patch and using latest gnulib, currently bootstrap fails. If you leave an hour or two between the gnulib and coreutils pushes, give me a heads up and I'll test the coreutils changes. But given the thorough coverage, and considering the relative portability, I'd be surprised to find a problem. ---------------------- There are two spacing nits in the src/mktemp.c change in "build: reflect gnulib changes to tempname": mkstemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_FILE, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_FILE, + suff_len); mkdtemp_len (char *tmpl, size_t suff_len, bool dry_run) { - return gen_tempname_len (tmpl, 0, dry_run ? GT_NOCREATE : GT_DIR, suff_len); + return gen_tempname_len (tmpl, 0, 0, dry_run ? GT_NOCREATE : GT_DIR, + suff_len); Those "return..." lines should be indented by only 2 spaces, not 3. ----------------------------------------------- The --help text reads better if it says "`X's", e.g., -TEMPLATE must end in at least 3 consecutive X.\n\ +TEMPLATE must end in at least 3 consecutive `X's.\n\ and in texinfo (though there may be more): +implied. The final run of @samp{X} in the @var{template} will be replaced -implied. The final run of @samp{X}s in the @var{template} will be replaced ------------------------------- BTW, I didn't cheat ;-), spotted the bug in your first patch, and see that you fixed it in the latest: - if (template[len - 1] != 'X') + if (!len || template[len - 1] != 'X') |
|
|
Re: [PATCHv2 0/7] Re: temp file suffixes: mktemp DWIM-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1 According to Jim Meyering on 11/5/2009 2:35 AM: > Nice work! > Modulo tiny comments, you're welcome to push that series > once you've pushed the gnulib bits upon which it depends. Aargh. I just noticed that what I pushed still has a bug: $ src/mktemp -u XXXXXXXXXXXXXXXXX.pdf XXXXXXXXXXXXXpiYD.pdf Rather than replacing all the X, it is only replacing the number of X in the suffix. And the testsuite didn't catch it :( I'll have to prepare a followup patch, but I ran out of time this morning. - -- Don't work too hard, make some time for fun as well! Eric Blake ebb9@... -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkry4zIACgkQ84KuGfSFAYB/nwCcDGr6BPtJJupywDG2h+iloBPj CcwAn1PUS1aAA3w8Yz6M+e8bZAh98vF7 =FAW2 -----END PGP SIGNATURE----- |
|
|
Re: [PATCHv2 0/7] Re: temp file suffixes: mktemp DWIMEric Blake wrote:
> According to Jim Meyering on 11/5/2009 2:35 AM: >> Nice work! >> Modulo tiny comments, you're welcome to push that series >> once you've pushed the gnulib bits upon which it depends. > > Aargh. I just noticed that what I pushed still has a bug: > > $ src/mktemp -u XXXXXXXXXXXXXXXXX.pdf > XXXXXXXXXXXXXpiYD.pdf > > Rather than replacing all the X, it is only replacing the number of X in > the suffix. And the testsuite didn't catch it :( > > I'll have to prepare a followup patch, but I ran out of time this morning. Here's at least part of it: diff --git a/src/mktemp.c b/src/mktemp.c index ac35026..f60e824 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -118,14 +118,14 @@ static int mkstemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_FILE, - suff_len); + x_len); } static int mkdtemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_DIR, - suff_len); + x_len); } int |
|
|
Re: [PATCHv2 0/7] Re: temp file suffixes: mktemp DWIMEric Blake <ebb9 <at> byu.net> writes:
> Aargh. I just noticed that what I pushed still has a bug: > > $ src/mktemp -u XXXXXXXXXXXXXXXXX.pdf > XXXXXXXXXXXXXpiYD.pdf > > Rather than replacing all the X, it is only replacing the number of X in > the suffix. And the testsuite didn't catch it :( But --enable-gcc-warnings did. | cc1: warnings being treated as errors | mktemp.c: In function ‘mkstemp_len’: | mktemp.c:118: error: unused parameter ‘x_len’ [-Wunused-parameter] | mktemp.c: In function ‘mkdtemp_len’: | mktemp.c:125: error: unused parameter ‘x_len’ [-Wunused-parameter] Here's what I'll push that once I get a chance to turn this into a proper patch. diff --git i/src/mktemp.c w/src/mktemp.c index ac35026..f60e824 100644 --- i/src/mktemp.c +++ w/src/mktemp.c @@ -118,14 +118,14 @@ static int mkstemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_FILE, - suff_len); + x_len); } static int mkdtemp_len (char *tmpl, size_t suff_len, size_t x_len, bool dry_run) { return gen_tempname_len (tmpl, suff_len, 0, dry_run ? GT_NOCREATE : GT_DIR, - suff_len); + x_len); } int |
| < Prev | 1 - 2 | Next > |
| Free embeddable forum powered by Nabble | Forum Help |