Re: [PATCH 3/3] reftable tests: avoid "int" overflow, use "uint64_t"

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 11, 2022, 19:28 UTC
Message-ID: <Yd3aZ5Q9Ph0H+Ijn@nand.local>
In-Reply-To: <patch-3.3-93112305523-20220111T163908Z-avarab@gmail.com>

On Tue, Jan 11, 2022 at 05:40:23PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> diff --git a/reftable/merged_test.c b/reftable/merged_test.c
> index 24461e8a802..b87ff495dfd 100644
> --- a/reftable/merged_test.c
> +++ b/reftable/merged_test.c
> @@ -24,8 +24,8 @@ license that can be found in the LICENSE file or at
>  static void write_test_table(struct strbuf *buf,
>  			     struct reftable_ref_record refs[], int n)
>  {
> -	int min = 0xffffffff;
> -	int max = 0;
> +	uint64_t min = 0xffffffff;
> +	uint64_t max = 0;

Han-Wen: it looks like the loop below the context here is to set the
min/max of update_index over all of the ref records?

If so, making these comparisons all unsigned makes sense to me. In practice it's probably fine at least from a signedness perspective, since the compiler _should_ be coercing both operands to unsigned.

But perhaps not so from a width perspective, if sizeof(int) != 8 (though I suspect in practice that we are unlikely to have enough possible values of update_index for that to matter).

In any case, you're only setting the lower half of `min` high. Maybe:

    uint64_t min = ~0ul;

instead?

Thanks, Taylor