On 2/5/21, Simon J. Gerraty <[email protected]> wrote: > Mateusz Guzik <[email protected]> wrote: > >> [External Email. Be cautious of content] >> >> >> Responding to the original mail due to several follow ups. >> >> Thanks for fixups so far. >> >> Another note related to JobOpenTmpFile: >> >> JobSigLock(&mask); >> tfd = mkTempFile(TMPPAT, &tfile); >> if (!DEBUG(SCRIPT)) >> (void)eunlink(tfile); >> JobSigUnlock(&mask); >> >> 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. >> Apart from that, eunlink stats the path and this has happens in >> default setting. Instead I propose the following: since mkTempFile has >> only 2 consumers, passing in the buffer can be made mandatory. Then >> another flag can control when the routine should unlink the result. >> This would shorten this callsite to roughly: >> >> char buf[MAXPATHLATEN]; >> ... >> tfd = mkTempFile(TMPPAT, buf, sizeof buf, !DEBUG(SCRIPT)); >> >> which will also save on memory alloc and string coying as the target >> routine calls bmake_strdup to accomodate the request. > > Will take a look. > -- Mateusz Guzik <mjguzik gmail.com>
