Subject: Re: [PATCH 1/6] maintenance: add get_random_minute()

From: Junio C Hamano <[email-masked]>
Date: Tue, 08 Aug 2023 09:49:30 -0700

Junio C Hamano <[email-masked]> writes:

> Provided if code simplification is a good enough rationale, the
> patch looks sensible, but I find its use of u64 a bit questionable
> (though not wrong).  I would have expected that the type of "rand"
> would be the same as backoff_ms and wait_ms, two variables involved
> in the same expression.

Ah, not so fast.  The use of modulo means we need to be careful
about about the fuzzing factor going negative, and use of unsigned
type allows us to forget about it.

(fuzz % 250), when fuzz is of a signed random integral type, ranges
between -250 and +250 and because we want the center of our
distribution at 1000, so I think the following is equivalent.

 lockfile.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git c/lockfile.c w/lockfile.c
index 6587d407f4..7de53526ac 100644
--- c/lockfile.c
+++ w/lockfile.c
@@ -115,9 +115,8 @@ static int lock_file_timeout(struct lock_file *lk, const char *path,
 		remaining_ms = timeout_ms;
 
 	while (1) {
-		long backoff_ms, wait_ms;
+		long backoff_ms, wait_ms, fuzz;
 		int fd;
-		uint64_t rand;
 
 		fd = lock_file(lk, path, flags, mode);
 
@@ -130,10 +129,10 @@ 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 */
-		if (csprng_bytes(&rand, sizeof(uint64_t)) < 0)
+		if (csprng_bytes(&fuzz, sizeof(fuzz)) < 0)
 			return error_errno(_("unable to get random bytes for"
 					     "lockfile backoff"));
-		wait_ms = (750 + rand % 500) * backoff_ms / 1000;
+		wait_ms = (1000 + fuzz % 250) * backoff_ms / 1000;
 		sleep_millisec(wait_ms);
 		remaining_ms -= wait_ms;