From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4160 invoked from network); 19 Oct 2004 05:38:02 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 19 Oct 2004 05:38:02 -0000 Received: (qmail 94976 invoked from network); 19 Oct 2004 05:37:56 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 19 Oct 2004 05:37:56 -0000 Received: (qmail 8872 invoked by alias); 19 Oct 2004 05:37:44 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 20505 Received: (qmail 8853 invoked from network); 19 Oct 2004 05:37:42 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 19 Oct 2004 05:37:42 -0000 Received: (qmail 94665 invoked from network); 19 Oct 2004 05:37:42 -0000 Received: from dsl3-63-249-88-2.cruzio.com (HELO binome.blorf.net) (63.249.88.2) by a.mx.sunsite.dk with SMTP; 19 Oct 2004 05:37:38 -0000 Received: by binome.blorf.net (Postfix, from userid 1000) id 9BF1F4F7C; Mon, 18 Oct 2004 22:37:36 -0700 (PDT) Date: Mon, 18 Oct 2004 22:37:36 -0700 From: Wayne Davison To: zsh-workers@sunsite.dk Subject: [PATCH] Improved temp-file creation Message-ID: <20041019053736.GA23313@blorf.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="sdtB3X0nJg68CQEu" Content-Disposition: inline User-Agent: Mutt/1.5.6+20040722i X-Spam-Checker-Version: SpamAssassin 2.63 on a.mx.sunsite.dk X-Spam-Level: X-Spam-Status: No, hits=0.0 required=6.0 tests=none autolearn=no version=2.63 X-Spam-Hits: 0.0 --sdtB3X0nJg68CQEu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline After my recent changes to gettempname() and its callers, I decided that it would be nice to have a similar function that works like mkstemp() (and actually uses it, when available). This way callers that are going to do an immediate open() with O_CREAT|O_EXCL can all use the same code, which will hopefully make it easier to ensure that callers don't use mktemp() in an unsafe manner. It also makes things more efficient when mkstemp() is available because that call can avoid the extra lstat() calls that mktemp() uses. The appended patch adds the function gettempfile() that takes the same (pretty new) parameters as gettempname(), plus an extra one that gets set to the pointer for the name (since the function returns a file descriptor). I then sprinkled around the new function into the code. One caller of gettempname() even got changed to ask for non-heap memory instead of calling ztrdup() on the returned heap string. ..wayne.. --sdtB3X0nJg68CQEu Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="temp-files.patch" --- Src/builtin.c 18 Oct 2004 19:07:50 -0000 1.130 +++ Src/builtin.c 19 Oct 2004 05:32:17 -0000 @@ -1445,10 +1445,8 @@ bin_fc(char *nam, char **argv, Options o char *fil; retval = 1; - fil = gettempname(NULL, 1); - if (((tempfd = open(fil, O_WRONLY | O_CREAT | O_EXCL | O_NOCTTY, 0600)) - == -1) || - ((out = fdopen(tempfd, "w")) == NULL)) { + if ((tempfd = gettempfile(NULL, 1, &fil)) < 0 + || ((out = fdopen(tempfd, "w")) == NULL)) { unqueue_signals(); zwarnnam("fc", "can't open temp file: %e", NULL, errno); } else { @@ -3535,8 +3533,8 @@ bin_print(char *name, char **args, Optio zwarnnam(name, "open_memstream failed", NULL, 0); #else int tempfd; - char *tmpf = gettempname(NULL, 1); - if ((tempfd = open(tmpf, O_RDWR|O_CREAT|O_EXCL, 0644)) < 0 + char *tmpf; + if ((tempfd = gettempfile(NULL, 1, &tmpf)) < 0 || (fout = fdopen(tempfd, "w+")) == NULL) zwarnnam(name, "can't open temp file: %e", NULL, errno); unlink(tmpf); --- Src/exec.c 18 Oct 2004 19:07:55 -0000 1.75 +++ Src/exec.c 19 Oct 2004 05:32:19 -0000 @@ -2801,8 +2801,7 @@ getherestr(struct redir *fn) untokenize(t); unmetafy(t, &len); t[len++] = '\n'; - s = gettempname(NULL, 1); - if (!s || (fd = open(s, O_CREAT|O_WRONLY|O_EXCL|O_NOCTTY, 0600)) == -1) + if ((fd = gettempfile(NULL, 1, &s)) < 0) return -1; write(fd, t, len); close(fd); @@ -2975,11 +2974,9 @@ getoutputfile(char *cmd) return NULL; if (!(prog = parsecmd(cmd))) return NULL; - if (!(nam = gettempname(NULL, 1))) + if (!(nam = gettempname(NULL, 0))) return NULL; - nam = ztrdup(nam); - if (!jobtab[thisjob].filelist) jobtab[thisjob].filelist = znewlinklist(); zaddlinknode(jobtab[thisjob].filelist, nam); --- Src/hist.c 18 Oct 2004 19:07:30 -0000 1.55 +++ Src/hist.c 19 Oct 2004 05:32:19 -0000 @@ -2137,8 +2137,7 @@ lockhistfile(char *fn, int keep_trying) lockfile = bicat(unmeta(fn), ".LOCK"); #ifdef HAVE_LINK - tmpfile = gettempname(fn, 0); - if ((fd = open(tmpfile, O_WRONLY|O_CREAT|O_EXCL, 0644)) >= 0) { + if ((fd = gettempfile(fn, 0, &tmpfile)) >= 0) { FILE *out = fdopen(fd, "w"); if (out) { fprintf(out, "%ld %s\n", (long)getpid(), getsparam("HOST")); @@ -2163,8 +2162,8 @@ lockhistfile(char *fn, int keep_trying) break; } unlink(tmpfile); + free(tmpfile); } - free(tmpfile); #else /* not HAVE_LINK */ while ((fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, 0644)) < 0) { if (errno != EEXIST || !keep_trying) --- Src/utils.c 18 Oct 2004 19:07:46 -0000 1.68 +++ Src/utils.c 19 Oct 2004 05:32:20 -0000 @@ -1156,6 +1156,47 @@ gettempname(const char *prefix, int use_ return ret; } +/**/ +mod_export int +gettempfile(const char *prefix, int use_heap, char **tempname) +{ + char *fn; + int fd; +#if HAVE_MKSTEMP + char *suffix = prefix ? ".XXXXXX" : "XXXXXX"; + + if (!prefix && !(prefix = getsparam("TMPPREFIX"))) + prefix = DEFAULT_TMPPREFIX; + if (use_heap) + fn = dyncat(unmeta(prefix), suffix); + else + fn = bicat(unmeta(prefix), suffix); + + fd = mkstemp(fn); + if (fd < 0) { + if (!use_heap) + free(fn); + fn = NULL; + } +#else + int failures = 0; + + do { + if (!(fn = gettempname(prefix, use_heap))) { + fd = -1; + break; + } + if ((fd = open(fn, O_RDWR | O_CREAT | O_EXCL, 0600)) >= 0) + break; + if (!use_heap) + free(fn); + fn = NULL; + } while (errno == EEXIST && ++failures < 16); +#endif + *tempname = fn; + return fd; +} + /* Check if a string contains a token */ /**/ --- Src/Modules/zftp.c 18 Oct 2004 19:07:56 -0000 1.33 +++ Src/Modules/zftp.c 19 Oct 2004 05:32:21 -0000 @@ -1971,8 +1971,7 @@ zftp_open(char *name, char **args, int f * However, it is closed whenever there are no connections open. */ if (zfstatfd == -1) { - fname = gettempname(NULL, 1); - zfstatfd = open(fname, O_RDWR|O_CREAT|O_EXCL, 0600); + zfstatfd = gettempfile(NULL, 1, &fname); DPUTS(zfstatfd == -1, "zfstatfd not created"); #if defined(F_SETFD) && defined(FD_CLOEXEC) /* If the shell execs a program, we don't want this fd left open. */ --sdtB3X0nJg68CQEu--