From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18974 invoked from network); 19 Feb 2003 05:38:26 -0000 Received: from sunsite.dk (130.225.247.90) by ns1.primenet.com.au with SMTP; 19 Feb 2003 05:38:26 -0000 Received: (qmail 5652 invoked by alias); 19 Feb 2003 05:38:10 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 18261 Received: (qmail 5645 invoked from network); 19 Feb 2003 05:38:10 -0000 Received: from localhost (HELO sunsite.dk) (127.0.0.1) by localhost with SMTP; 19 Feb 2003 05:38:10 -0000 X-MessageWall-Score: 0 (sunsite.dk) Received: from [65.206.41.66] by sunsite.dk (MessageWall 1.0.8) with SMTP; 19 Feb 2003 5:38:9 -0000 Received: from gauss.counterexample.org (gauss.counterexample.org [192.168.13.4]) by ns00.counterexample.org (8.12.5/8.11.2) with ESMTP id h1J5cBhx010770; Wed, 19 Feb 2003 00:38:11 -0500 Received: (from guthrie@localhost) by gauss.counterexample.org (8.11.6/8.11.6) id h1J5cBf15117; Wed, 19 Feb 2003 00:38:11 -0500 From: "John T. Guthrie" Message-Id: <200302190538.h1J5cBf15117@gauss.counterexample.org> Subject: Re: PATCH: small compilation cleanup for utils.c To: zsh-workers@sunsite.dk Date: Wed, 19 Feb 2003 00:38:11 -0500 (EST) Cc: guthrie@counterexample.org In-Reply-To: <20030218091358.GA29696@fysh.org> from "Zefram" at Feb 18, 2003 09:13:58 AM X-Mailer: ELM [version 2.5 PL6] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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@counterexample.org -----------------------------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;