Hi,
>>> + zend_long rand;
>>> + php_random_int(1000000000, 9999999999, &rand, 1);
>>> + uniqid = strpprintf(0, "%s%08x%05x%.8F", prefix, sec,
>>> usec, (double)rand/10000000000);
>>
>> Your code is broken. It produces 0.10000000 - 0.99999999 when it should
>> produce 0.00000000 - 9.99999999. Also, you have integer overflow on 32-bit
>> systems.
>>
>> Why do you mess with oversized integers and doubles and at all? It would
>> be cleaner and simpler to use just regular 32-bit integers like this:
>>
>> + zend_long rand;
>> + php_random_int(0, 999999999, &rand, 1);
>> + uniqid = strpprintf(0, "%s%08x%05x%d.%08d", prefix, sec,
>> usec, rand % 10, rand / 10);
...
> My original patch is better, then,
>
> [yohgaki@dev PHP-master]$ git show 48f1a17886d874dc90867c669481804de90
> commit 48f1a17886d874dc90867c669481804de90509e8
> Author: Yasuo Ohgaki <yohg...@php.net>
> Date: Tue Oct 18 09:04:57 2016 +0900
>
> Fix bug #47890 #73215 uniqid() should use better random source
>
> diff --git a/ext/standard/uniqid.c b/ext/standard/uniqid.c
> index f429e6d..207cf01 100644
> --- a/ext/standard/uniqid.c
> +++ b/ext/standard/uniqid.c
> @@ -35,9 +35,11 @@
> #include <sys/time.h>
> #endif
>
> -#include "php_lcg.h"
> +#include "php_random.h"
> #include "uniqid.h"
>
> +#define PHP_UNIQID_ENTROPY_LEN 10
> +
> /* {{{ proto string uniqid([string prefix [, bool more_entropy]])
> Generates a unique ID */
> #ifdef HAVE_GETTIMEOFDAY
> @@ -77,7 +79,22 @@ PHP_FUNCTION(uniqid)
> * digits for usecs.
> */
> if (more_entropy) {
> - uniqid = strpprintf(0, "%s%08x%05x%.8F", prefix, sec, usec,
> php_combined_lcg() * 10);
> + int i;
> + unsigned char c, entropy[PHP_UNIQID_ENTROPY_LEN+1];
> +
> + for(i = 0; i < PHP_UNIQID_ENTROPY_LEN;) {
> + php_random_bytes_throw(&c, sizeof(c));
> + /* Avoid modulo bias */
> + if (c > 249) {
> + continue;
> + }
> + entropy[i] = c % 10 + '0';
> + i++;
> + }
> + /* Set . for compatibility */
> + entropy[1] = '.';
> + entropy[PHP_UNIQID_ENTROPY_LEN] = '\0';
> + uniqid = strpprintf(0, "%s%08x%05x%s", prefix, sec, usec,
> entropy);
> } else {
> uniqid = strpprintf(0, "%s%08x%05x", prefix, sec, usec);
> }
>
>
> I'll revert the revert commit to master.
No. Lauri's version is better.
Your php_random_bytes_throw() version is significantly slow. Lauri's
version is faster and cleaner.
[original uniqid() using php_combined_lcg]
$ time ./php_uniqid_orig -r 'for($i=0; $i<1000000;$i++) uniqid("",true);'
real 0m0.366s
user 0m0.350s
sys 0m0.010s
[your php_random_bytes_throw version (commit
48f1a17886d874dc90867c669481804de90509e8)]
$ time ./php_uniqid_yohgaki -r 'for($i=0; $i<1000000;$i++) uniqid("",true);'
real 0m4.509s
user 0m0.430s
sys 0m4.070s
[Lauri's php_random_int version]
$ time ./php_uniqid_lauri -r 'for($i=0; $i<1000000;$i++) uniqid("",true);'
real 0m0.664s
user 0m0.260s
sys 0m0.400s
--
Kazuo Oishi
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
