cvs commit: src/usr.bin/make job.c

[Max Okumoto]

Giorgos Keramidas <keramida at freebsd.org> writes:

> On 2005-05-18 01:41, Max Okumoto <okumoto at ucsd.edu> wrote:
>> Your idea of using mkdtemp() can be fixed by putting a loop
>> around the code. Each time around the loop would be expensive
>> but we wouldn't be doing that to often anyway.
>>
>> loop:
>>     mkdtemp(template)
>>     mkfifo(tempalte + "/fifo")
>>     if error remove temp directory, restore template and loop.
>>
>> Or better yet, if someone could create an equiv function in libc
>> so I don't have to maintain it in make(1) :-)  Do any other
>> programs need the ability to make a temp fifo?
>>
>> Personally, I don't think it is a risk, but I wanted other
>> peoples opinions, before I tried to fix a non-issue. :-)
>
> Does this really need to be of the form DIR/fifo ?
>
> I haven't looked at the code that uses the fifo at all, so I risk being
> extremely out of topic here, but why wouldn't a temporary fifo created
> with a name obtained from mkstemp() work too?

I think you mean mktemp(), since mkstemp() actucally creates a file
and returns a file descriptor.  The reason that I rewrote the code
to obtain a fifo file was that the libc generates a warnning message
when you link with mktemp().

warning: mktemp() possibly used unsafely; consider using mkstemp()

As part of the refactoring of the  make(1) that I am doing, I am
correcting all the warnings.  The original code that phk committed
generated this messages and had the race condition.  The current code 
which is derived from mkstemp(), got rid of the error message and
the race condition.  But has the disadvantage that the code is
pretty much a duplicate of the original libc code, sitting in
/src/usr.bin/make/job.c:mkfifotemp() which is 80 lines. 

Alexander suggested that I replace that code with
    mkdtemp(template)
    mkfifo(tempalte + "/fifo")

Which removed alot of the code duplication, but added the race
back in.  So my question is it this race something that I
should worry about?  IMHO removing the warning message was
important, but the race isn't much of a problem, but would
like some input on it.

Thanks
                        Max

>
> A directory won't be needed if the fifo name is created by mkstemp() and
> then passed directly to mkfifo(2).
>
> Then there is still a (small?) possibility for a race, but a subsequent
> invocation of mkstemp() is almost guaranteed to work, unless mkstemp()
> is severely broken.