[llvm-commits] [llvm] r156240 - /llvm/trunk/lib/Support/Process.cpp

[Daniel Dunbar]

On Tue, May 8, 2012 at 11:27 AM, Chandler Carruth <chandlerc at google.com> wrote:
> On Tue, May 8, 2012 at 11:19 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>>
>> On Sun, May 6, 2012 at 1:46 AM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>> > On Sat, May 5, 2012 at 10:36 AM, Duncan Sands <baldrick at free.fr> wrote:
>> >>
>> >> > Won't this give us really bad random numbers on system with no
>> >> > arc4random (linux)? I'm thinking of multiple invocations of clang in
>> >> > the
>> >> > same second all getting the same "random" file name. Does Linux have
>> >> > another
>> >> > source of pseudorandom numbers?
>> >>
>> >> If this is about getting unique temporary files, shouldn't tmpfile or
>> >> mkstemp be
>> >> used?
>> >
>> >
>> > Yes, and it seems to have caused: PR12743
>> >
>> > This is *not* the way to get temporary files. There are proper OS APIs
>> > for
>> > that we should be using, not RNGs....
>>
>> I tend to agree, however this is not what I changed. I just replaced
>> factored out the horribleness of how the RNG was used (both
>> implementations were poor, for different reasons). The seeding problem
>> should be fixed now, thanks to chapuni's r156252.
>>
>> I would support someone eliminating the current API and using the
>> proper OS API, but it requires changing the fs API because of the way
>> unique_file was defined.
>>
>> That said, why is using the OS API so much better than using a well
>> seeded RNG? I'm not familiar with the Linux implementation.
>
>
> AFAIK, it actually resolves conflicts, and may be cheaper than getting a
> well seeded RNG. that said, I've not read the implementation in detail, it
> just seems that the reliable way is to fall back on OS interfaces rather
> than rolling our own....

Ok, my guess is the current implementation isn't much worse (if at
all). On Darwin they are effectively equivalent. The OS mkstemp()
interface has a slight different template style and our implementation
is simple enough that I'm not personally motivated to make the API
change.

There are also pro's to having our own implementation, the OS
implementation doesn't allow specifying mode bits for example, which
leads to stupid code like we currently have in Unix/Path.inc's
AddPermissionBits().

FWIW, I'm committing another fix in a sec to make sure we always get a
decent seed in the RNG.

 - Daniel