Re: make: don't use bogus $TMPDIR

["Simon J. Gerraty" <sjg%crufty.net@localhost>]

On Thu, 22 Apr 2010 17:54:02 +0000, David Holland writes:
>On Thu, Apr 22, 2010 at 10:50:36AM -0700, Simon J. Gerraty wrote:
> >> Looks reasonable, except that if it were me I'd centralize the code
> >> for setting up a temporary file, and make the tmpdir variable private
> >> there.
> > 
> > The other place I use tmpDir is in the meta mode stuff - which I'll be
> > following up on in due course.
> > I could make the change you suggest easily enough.
>
>It seems worthwhile, particularly since temporary file construction is
>one of those things that needs to be audited now and then.

Agreed - see the patch below.
If you don't object to the .MAKE.JOBS patch - even better ;-)

Index: job.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/job.c,v
retrieving revision 1.147
diff -u -p -r1.147 job.c
--- job.c       7 Apr 2010 00:11:27 -0000       1.147
+++ job.c       22 Apr 2010 18:14:46 -0000
@@ -342,8 +342,6 @@ static sigset_t caught_signals;     /* Set o
 #define KILLPG(pid, sig)       killpg((pid), (sig))
 #endif
 
-static char *tmpdir;           /* directory name, always ending with "/" */
-
 static void JobChildSig(int);
 static void JobContinueSig(int);
 static Job *JobFindPid(int, int);
@@ -1555,11 +1553,7 @@ JobStart(GNode *gn, int flags)
        }
 
        JobSigLock(&mask);
-       tfile = bmake_malloc(strlen(tmpdir) + sizeof(TMPPAT));
-       strcpy(tfile, tmpdir);
-       strcat(tfile, TMPPAT);
-       if ((tfd = mkstemp(tfile)) == -1)
-           Punt("Could not create temporary file %s", strerror(errno));
+       tfd = mkTempFile(TMPPAT, &tfile);
        if (!DEBUG(SCRIPT))
                (void)eunlink(tfile);
        JobSigUnlock(&mask);
@@ -2122,8 +2116,6 @@ void
 Job_Init(void)
 {
     GNode         *begin;     /* node for commands to do at the very start */
-    const char    *p;
-    size_t        len;
 
     /* Allocate space for all the job info */
     job_table = bmake_malloc(maxJobs * sizeof *job_table);
@@ -2136,18 +2128,6 @@ Job_Init(void)
 
     lastNode =   NULL;
 
-    /* set tmpdir, and ensure that it ends with "/" */
-    p = getenv("TMPDIR");
-    if (p == NULL || *p == '\0') {
-       p = _PATH_TMP;
-    }
-    len = strlen(p);
-    tmpdir = bmake_malloc(len + 2);
-    strcpy(tmpdir, p);
-    if (tmpdir[len - 1] != '/') {
-       strcat(tmpdir, "/");
-    }
-
     if (maxJobs == 1) {
        /*
         * If only one job can run at a time, there's no need for a banner,
Index: main.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/main.c,v
retrieving revision 1.179
diff -u -p -r1.179 main.c
--- main.c      20 Apr 2010 17:18:08 -0000      1.179
+++ main.c      22 Apr 2010 18:14:46 -0000
@@ -385,6 +385,7 @@ rearg:      
                case 'B':
                        compatMake = TRUE;
                        Var_Append(MAKEFLAGS, "-B", VAR_GLOBAL);
+                       Var_Set(MAKE_MODE, "compat", VAR_GLOBAL, 0);
                        break;
                case 'C':
                        if (chdir(argvalue) == -1) {
@@ -500,6 +501,7 @@ rearg:      
                        }
                        Var_Append(MAKEFLAGS, "-j", VAR_GLOBAL);
                        Var_Append(MAKEFLAGS, argvalue, VAR_GLOBAL);
+                       Var_Set(".MAKE.JOBS", argvalue, VAR_GLOBAL, 0);
                        maxJobTokens = maxJobs;
                        break;
                case 'k':
@@ -1942,3 +1944,46 @@ Main_ExportMAKEFLAGS(Boolean first)
 #endif
     }
 }
+
+/*
+ * Create and open a temp file using "pattern".
+ * If "fnamep" is provided set it to a copy of the filename created.
+ * Otherwise unlink the file once open.
+ */
+int
+mkTempFile(const char *pattern, char **fnamep)
+{
+    static char *tmpdir = NULL;
+    char tfile[MAXPATHLEN];
+    int fd;
+    
+    if (!pattern)
+       pattern = TMPPAT;
+
+    if (!tmpdir) {
+       struct stat st;
+
+       /*
+        * Honor $TMPDIR but only if it is valid.
+        * Ensure it ends with /.
+        */
+       tmpdir = Var_Subst(NULL, "${TMPDIR:tA:U/tmp}/", VAR_GLOBAL, 0);
+       if (stat(tmpdir, &st) < 0 || !S_ISDIR(st.st_mode)) {
+           free(tmpdir);
+           tmpdir = bmake_strdup(_PATH_TMP);
+       }
+    }
+    if (pattern[0] == '/') {
+       strlcpy(tfile, pattern, sizeof(tfile));
+    } else {
+       snprintf(tfile, sizeof(tfile), "%s%s", tmpdir, pattern);
+    }
+    if ((fd = mkstemp(tfile)) < 0)
+       Punt("Could not create temporary file %s: %s", tfile, strerror(errno));
+    if (fnamep) {
+       *fnamep = bmake_strdup(tfile);
+    } else {
+       unlink(tfile);                  /* we just want the descriptor */
+    }
+    return fd;
+}
Index: make.h
===================================================================
RCS file: /cvsroot/src/usr.bin/make/make.h,v
retrieving revision 1.80
diff -u -p -r1.80 make.h
--- make.h      7 Apr 2010 00:11:27 -0000       1.80
+++ make.h      22 Apr 2010 18:14:46 -0000
@@ -450,6 +450,7 @@ void Check_Cwd(const char **);
 void PrintOnError(GNode *, const char *);
 void Main_ExportMAKEFLAGS(Boolean);
 Boolean Main_SetObjdir(const char *);
+int mkTempFile(const char *, char **);
 
 #ifdef __GNUC__
 #define UNCONST(ptr)   ({              \