bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: temp file suffixes: mktemp DWIM


From: Eric Blake
Subject: Re: temp file suffixes: mktemp DWIM
Date: Wed, 4 Nov 2009 18:42:14 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

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.

      [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 <address@hidden>
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 <address@hidden>
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 address@hidden
+ #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 <address@hidden>
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 <address@hidden>
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 @address@hidden potential file names.
@@ -12108,6 +12108,15 @@ mktemp invocation
 @end example

 @item
+Create a temporary file with a known suffix.
address@hidden
+$ mktemp --suffix=.txt file-XXXX
+file-H08W.txt
+$ mktemp file-XXXX-XXXX.txt
+file-XXXX-eI9L.txt
address@hidden example
+
address@hidden
 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.

address@hidden address@hidden
address@hidden --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
address@hidden is inferred by finding the last @samp{X} in
address@hidden  This option exists for use with the default
address@hidden and for the creation of a @var{suffix} that starts with
address@hidden
+
 @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







reply via email to

[Prev in Thread] Current Thread [Next in Thread]