Re: [PATCH 2/4] repack.c: LLP64 compatibility, upcast unity for left shift

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 30, 2021, 00:35 UTC
Message-ID: <YaVxwFuMeK3QsfZj@nand.local>
In-Reply-To: <20211126113614.709-3-philipoakley@iee.email>

On Fri, Nov 26, 2021 at 11:36:12AM +0000, Philip Oakley wrote:

> Visual Studio reports C4334 "was 64-bit shift intended" warning
> because of size miss-match.

s/miss-/mis

Show 10 quoted lines

> diff --git a/builtin/repack.c b/builtin/repack.c
> index 0b2d1e5d82..6da66474fd 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -842,7 +842,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  			fname_old = mkpathdup("%s-%s%s",
>  					packtmp, item->string, exts[ext].name);
>
> -			if (((uintptr_t)item->util) & (1 << ext)) {
> +			if (((uintptr_t)item->util) & ((uintptr_t)1 << ext)) {

This line blames to me from back in 2fcb03b52d (builtin/repack.c: don't move existing packs out of the way, 2020-11-17).

The proposed fix here looks good to me (though we were never at any practical risk of getting bitten by a down-cast here since the maximum value for `ext` is 5).

Thanks, Taylor