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

From: Philip Oakley <philipoakley@iee.email>
Date: Nov 30, 2021, 22:28 UTC
Message-ID: <13a5828d-596f-a215-7053-9570be939e5b@iee.email>
In-Reply-To: <YaVxwFuMeK3QsfZj@nand.local>

On 30/11/2021 00:35, Taylor Blau wrote:

> 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

Thanks. Looks like "miss-pelling" is my blind spot. Will fix V2 soon.

Show 17 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).

Agreed. It's nice to get a head start on fixing a group of warnings.

> Thanks,
> Taylor

Philip