Subject: Re: [PATCH mh/lockfile-retry] lockfile: replace random() by rand()

From: Johannes Sixt <[email-masked]>
Date: Thu, 04 Jun 2015 10:40:48 +0200

Am 30.05.2015 um 19:12 schrieb Junio C Hamano:
> Johannes Sixt <[email-masked]> writes:
>
>> There you have it: Look the other way for a while, and people start
>> using exotic stuff... ;)
>
> Is it exotic to have random/srandom?  Both are in POSIX and 4BSD;
> admittedly rand/srand are written down in C89 and later, so they
> might be more portable, but I recall the prevailing wisdom is to
> favor random over rand for quality of randomness and portability, so
> I am wondering if it may be a better approach to keep the code as-is
> and do a compat/random.c based on either rand/srand (or use posix
> sample implementation [*1*]).

For our purposes here, the linear congruence of rand() is certainly 
sufficient. At this time, compatibility functions for random/srandom 
would just mean a lot of work for little gain.

>
>
> [Reference]
>
> *1* http://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html
>
>
>
>>
>> This is a build breakage of master on Windows. There are also a few
>> new test suite failures. On of them is in t1404#2, indicating that
>> a DF conflict takes a different error path. I haven't debugged, yet.
>> The lock file retry test fails, too. I'll report back as time permits.
>>
>>   lockfile.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lockfile.c b/lockfile.c
>> index 5a93bc7..ee5cb01 100644
>> --- a/lockfile.c
>> +++ b/lockfile.c
>> @@ -191,7 +191,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path,
>>   		return lock_file(lk, path, flags);
>>
>>   	if (!random_initialized) {
>> -		srandom((unsigned int)getpid());
>> +		srand((unsigned int)getpid());
>>   		random_initialized = 1;
>>   	}
>>
>> @@ -218,7 +218,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path,
>>
>>   		backoff_ms = multiplier * INITIAL_BACKOFF_MS;
>>   		/* back off for between 0.75*backoff_ms and 1.25*backoff_ms */
>> -		wait_us = (750 + random() % 500) * backoff_ms;
>> +		wait_us = (750 + rand() % 500) * backoff_ms;
>>   		sleep_microseconds(wait_us);
>>   		remaining_us -= wait_us;
>

-- Hannes