mktemp() generates completely deterministic filenames

[david.given]

mktemp() uses rand() to generate filenames, but rand() doesn't get seeded on process startup. This means that mktemp() will generate precisely the same sequence of filenames on every run.

While this is technically correct, which is the best kind of correct, it's possibly the worst thing it could possibly do --- it _looks_ like it's working, but doesn't. It pretty much guarantees file collisions with parallel builds.

I have since changed my code to use mkstemp() instead, which solves my particular issue, but Haiku's mktemp() shouldn't be doing this. I know that mktemp() is broken by design, but there's so much old software using it that we should really by trying harder. I'd suggest on of:

• use a real random number generator;

• keep rand(), but use a private seed based on the pid and timestamp;

• don't use random numbers at all and start with a constant string before mutating it! It's still technically correct, but now it's at least completely obvious what's happening.

Test program demonstrating the issue follows:

#include <stdlib.h>
#include <stdio.h>
int main() {
  char s[] = "/tmp/XXXXXX";
  mktemp(s);
  printf("%s\n", s);
}

$ gcc test.c
$ ./test
/tmp/temp.mq2NP5
$ ./test
/tmp/temp.mq2NP5
$ ./test
/tmp/temp.mq2NP5
$ ./test
/tmp/temp.mq2NP5

[phoudoin]

Nice catch, thanks for reporting it.

[phoudoin]

Fixed in hrev51354.

[phoudoin]

I take the private seed road.

[david.given]

Sorry --- that's not going to work!

• by calling srand() you're overwriting any seed used by the user, so disturbing any random number sequences they've set up.

• gettimeofday() is not guaranteed to return unique values. If the user calls mktemp() twice in very short succession, with an interval smaller than the timer granularity, then you end up seeding the random number generator twice with the same value, which means the two calls to mktemp() will generate the same filename.

I'd do something like this:

static int seed = 0; // value maintained between calls
if (seed == 0) {
  // seed the PRNG once on first call
  struct timeval tv;
  gettimeofday(&tv, 0);
  seed = (getpid() << 16) ^ getuid() ^ tv.tv_sec ^ tv.tv_usec;
}
...
uint32_t value = rand_r(&seed) % (sizeof(padchar) - 1);

[phoudoin]

Indeed, PulkoMandy said the same too. I will update the fix tomorrow except if somebody do it before.

[phoudoin]

Applied your patch in hrev51356. Thanks.

[david.given]

Thanks!