Re: make: don't use bogus $TMPDIR

[christos%astron.com@localhost (Christos Zoulas)]

In article <20100422073612.4E4A51C7053%bad.crufty.net@localhost>,
Simon J. Gerraty <sjg%crufty.net@localhost> wrote:
>Job_Init was using $TMPDIR from the environment, but that doesn't
>ensure it is valid....
>
>I've a need for this in more than one place so making tmpDir a global.
>
>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 07:33:18 -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,8 +1553,8 @@ JobStart(GNode *gn, int flags)
>       }
> 
>       JobSigLock(&mask);
>-      tfile = bmake_malloc(strlen(tmpdir) + sizeof(TMPPAT));
>-      strcpy(tfile, tmpdir);
>+      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));
>@@ -2122,8 +2120,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 +2132,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 07:33:18 -0000
>@@ -185,6 +185,7 @@ static char curdir[MAXPATHLEN + 1];        /* s
> static char objdir[MAXPATHLEN + 1];   /* where we chdir'ed to */
> char *progname;                               /* the program name */
> char *makeDependfile;
>+char *tmpDir;
> 
> Boolean forceJobs = FALSE;
> 
>@@ -385,6 +386,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 +502,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':
>@@ -1131,6 +1134,16 @@ main(int argc, char **argv)
>       if (p1)
>           free(p1);
> 
>+      /*
>+       * Honor $TMPDIR but only if it is valid.
>+       * Ensure it ends with /.
>+       */
>+      tmpDir = Var_Subst(NULL, "${TMPDIR:tA:U/tmp}/", VAR_GLOBAL, 0);

Looks good, but:
                "${TMPDIR:tA:U" _PATH_TMP "}"

christos