Re: [PATCH v2 2/2] Prevent git from rehashing 4GiB files

From: Jeff King <peff@peff.net>
Date: Oct 17, 2023, 00:00 UTC
Message-ID: <20231017000019.GB551672@coredump.intra.peff.net>
In-Reply-To: <20231012160930.330618-3-sandals@crustytoothpaste.net>

On Thu, Oct 12, 2023 at 04:09:30PM +0000, brian m. carlson wrote:

Show 12 quoted lines

> +static unsigned int munge_st_size(off_t st_size) {
> +	unsigned int sd_size = st_size;
> +
> +	/*
> +	 * If the file is an exact multiple of 4 GiB, modify the value so it
> +	 * doesn't get marked as racily clean (zero).
> +	 */
> +	if (!sd_size && st_size)
> +		return 0x80000000;
> +	else
> +		return sd_size;
> +}

Coverity complained that the "true" side of this conditional is unreachable, since sd_size is assigned from st_size, so the two values cannot be both true and false. But obviously we are depending here on the truncation of off_t to "unsigned int". I'm not sure if Coverity is just dumb, or if it somehow has a different size for off_t.

I don't _think_ this would ever cause confusion in a real compiler, as assignment from a larger type to a smaller has well-defined truncation, as far as I know.

But I do wonder if an explicit "& 0xFFFFFFFF" would make it more obvious what is happening (which would also do the right thing if in some hypothetical platform "unsigned int" ended up larger than 32 bits).

-Peff