Mateusz Guzik <[email protected]> wrote:
> >> is the signal stuff really necessary?
> >
> > It avoids the need to loop dealing with interupts.
> > The name seems missleading should be block not lock.
> >
>
> I know what it does, it popped up on truss.
>
> My point is that there are no callers of mkTempFile from a signal
> handler (and would argue if any were present, they should be fixed)
> and the DEBUG stuff seems to only be set at startup time. iow i don't
> see any relevance of signal handling to this code.
The open in mkstemp can fail due to EINTR.
That's the only reason I see for blocking signals.
The patch below eliminates the strdup in mkTempFile
and adds a Job_TempFile so meta.c can get the signal blocking too when
needed.
diff -r 19c080e3f296 bmake/job.c
--- a/bmake/job.c Mon Feb 01 10:44:48 2021 -0800
+++ b/bmake/job.c Fri Feb 05 10:16:20 2021 -0800
@@ -1586,8 +1586,7 @@
* are put. It is removed before the child shell is executed,
* unless DEBUG(SCRIPT) is set.
*/
- char *tfile;
- sigset_t mask;
+ char tfile[MAXPATHLEN];
int tfd; /* File descriptor to the temp file */
/*
@@ -1599,11 +1598,9 @@
DieHorribly();
}
- JobSigLock(&mask);
- tfd = mkTempFile(TMPPAT, &tfile);
+ tfd = Job_TempFile(TMPPAT, tfile, sizeof tfile);
if (!DEBUG(SCRIPT))
- (void)eunlink(tfile);
- JobSigUnlock(&mask);
+ eunlink(tfile);
job->cmdFILE = fdopen(tfd, "w+");
if (job->cmdFILE == NULL)
@@ -1620,8 +1617,6 @@
#endif
*out_run = JobPrintCommands(job);
-
- free(tfile);
}
/*
@@ -2796,6 +2791,20 @@
continue;
}
+/* Get a temp file */
+int
+Job_TempFile(const char *pattern, char *tfile, size_t tfile_sz)
+{
+ int fd;
+ sigset_t mask;
+
+ JobSigLock(&mask);
+ fd = mkTempFile(pattern, tfile, tfile_sz);
+ JobSigUnlock(&mask);
+
+ return fd;
+}
+
/* Prep the job token pipe in the root make process. */
void
Job_ServerStart(int max_tokens, int jp_0, int jp_1)
diff -r 19c080e3f296 bmake/job.h
--- a/bmake/job.h Mon Feb 01 10:44:48 2021 -0800
+++ b/bmake/job.h Fri Feb 05 10:16:20 2021 -0800
@@ -205,5 +205,6 @@
void Job_SetPrefix(void);
Boolean Job_RunTarget(const char *, const char *);
void Job_FlagsToString(const Job *, char *, size_t);
+int Job_TempFile(const char *, char *, size_t);
#endif /* MAKE_JOB_H */
diff -r 19c080e3f296 bmake/main.c
--- a/bmake/main.c Mon Feb 01 10:44:48 2021 -0800
+++ b/bmake/main.c Fri Feb 05 10:16:20 2021 -0800
@@ -2245,27 +2245,29 @@
* Otherwise unlink the file once open.
*/
int
-mkTempFile(const char *pattern, char **out_fname)
+mkTempFile(const char *pattern, char *tfile, size_t tfile_sz)
{
static char *tmpdir = NULL;
- char tfile[MAXPATHLEN];
+ char tbuf[MAXPATHLEN];
int fd;
if (pattern == NULL)
pattern = TMPPAT;
if (tmpdir == NULL)
tmpdir = getTmpdir();
+ if (tfile == NULL) {
+ tfile = tbuf;
+ tfile_sz = sizeof tbuf;
+ }
if (pattern[0] == '/') {
- snprintf(tfile, sizeof tfile, "%s", pattern);
+ snprintf(tfile, tfile_sz, "%s", pattern);
} else {
- snprintf(tfile, sizeof tfile, "%s%s", tmpdir, pattern);
+ snprintf(tfile, tfile_sz, "%s%s", tmpdir, pattern);
}
if ((fd = mkstemp(tfile)) < 0)
Punt("Could not create temporary file %s: %s", tfile,
strerror(errno));
- if (out_fname != NULL) {
- *out_fname = bmake_strdup(tfile);
- } else {
+ if (tfile == tbuf) {
unlink(tfile); /* we just want the descriptor */
}
return fd;
diff -r 19c080e3f296 bmake/make.h
--- a/bmake/make.h Mon Feb 01 10:44:48 2021 -0800
+++ b/bmake/make.h Fri Feb 05 10:16:20 2021 -0800
@@ -727,7 +727,7 @@
void PrintOnError(GNode *, const char *);
void Main_ExportMAKEFLAGS(Boolean);
Boolean Main_SetObjdir(Boolean, const char *, ...) MAKE_ATTR_PRINTFLIKE(2, 3);
-int mkTempFile(const char *, char **);
+int mkTempFile(const char *, char *, size_t);
int str2Lst_Append(StringList *, char *);
int getInt(const char *, int);
void GNode_FprintDetails(FILE *, const char *, const GNode *, const char *);
diff -r 19c080e3f296 bmake/meta.c
--- a/bmake/meta.c Mon Feb 01 10:44:48 2021 -0800
+++ b/bmake/meta.c Fri Feb 05 10:16:20 2021 -0800
@@ -144,7 +144,10 @@
* cwd causing getcwd to do a lot more work.
* We only care about the descriptor.
*/
- pbm->mon_fd = mkTempFile("filemon.XXXXXX", NULL);
+ if (!opts.compatMake)
+ pbm->mon_fd = Job_TempFile("filemon.XXXXXX", NULL, 0);
+ else
+ pbm->mon_fd = mkTempFile("filemon.XXXXXX", NULL, 0);
if ((dupfd = dup(pbm->mon_fd)) == -1) {
err(1, "Could not dup filemon output!");
}
