Re: PATCH: small compilation cleanup for utils.c

["John T. Guthrie" <guthrie@xxxxxxxxxxxxxxxxxx>]

Zefram wrote:
> John T. Guthrie wrote:
> >+     ret = ((char *) mkstemp(dyncat(unmeta(s), "XXXXXX")));
> 
> That's not how to use mkstemp().  It returns a file descriptor, not
> the pathname.
> 
> -zefram

Mea Culpa!  I guess that is what I get for doing this at 3:30 in the morning,
and then not actually checking that it worked.  (Which 3:30AM may also have
something to do with.)  So anyway, here is a patch that actually uses
mkstemp() correctly, and I've actually verified that it works by using
the =(list) construct.

That said, I have to wonder if this patch is even
worth the trouble.  :-(  I say that because mkstemp actually creates and opens
the file for you.  In every place in the code where gettempname() is
called, the shell subsequently opens the file.  Since dangling file
descriptors is normally considered a bad thing(TM), we want to close the
file descriptor that we are given.  Normally, we could just leave things
this way, and let the shell re-open the file after gettempname() returns.
However, in namedpipe() in exec.c, there is a call to gettempname() that
is immediately followed by either a mknod() or a mkfifo().  These will
fail if the file already exists as a plain file.  Thus, we either have to
unlink() the temporary file that we just created, or add in some more 
intelligence to gettempname() so that it knows something about who is
calling it.  For the time being, I have chosen the simpler path of just
deleting the file, and letting the shell re-create the file later on, but
doesn't this really eliminate alot of the benefit of using mkstemp() in
the first place.

That said, here is a correct version of the patch.

John Guthrie
guthrie@xxxxxxxxxxxxxxxxxx

-----------------------------CUT HERE-------------------------------

*** utils.c.orig        2002-10-15 14:00:00.000000000 -0400
--- utils.c     2003-02-18 23:38:41.000000000 -0500
***************
*** 1103,1119 ****
--- 1103,1130 ----
  gettempname(void)
  {
      char *s, *ret;
+ #ifdef HAVE_MKSTEMP
+     int fd;
+     char *t;
+ #endif

      queue_signals();
      if (!(s = getsparam("TMPPREFIX")))
        s = DEFAULT_TMPPREFIX;

+ #ifdef HAVE_MKSTEMP
+     fd = mkstemp(t = dyncat(unmeta(s), "XXXXXX"));
+     close(fd);
+     unlink(t);
+     ret = t;
+ #else
  #ifdef HAVE__MKTEMP
      /* Zsh uses mktemp() safely, so silence the warnings */
      ret = ((char *) _mktemp(dyncat(unmeta(s), "XXXXXX")));
  #else
      ret = ((char *) mktemp(dyncat(unmeta(s), "XXXXXX")));
  #endif
+ #endif
      unqueue_signals();

      return ret;