mbox series

[0/4] Fix LLP64 `(size_t)1` compatibility VS C4334 warnings

Message ID 20211126113614.709-1-philipoakley@iee.email (mailing list archive)
Headers show
Series Fix LLP64 `(size_t)1` compatibility VS C4334 warnings | expand

Message

Philip Oakley Nov. 26, 2021, 11:36 a.m. UTC
The Visual Studio MSVC compilation reports a number of C4334 "was 64-bit
shift intended" size mismatch warnings. In most of these cases a size_t
is ANDed (masked) with a bit shift of 1, or 1U. On LLP64 systems the unity
value is 32 bits, while size_t is 64 bits. 

The fix is to upcast the unity value to size_t.   

The first patch has also been reported [1] by René Scharfe as an extra patch
to the rs/mergesort series. That patch had been on maint.

The middle two patches are similar changes, though [2/4] is a uintptr_t.

The final patch is applied to object-file.c, which has recently been
renamed from sha1-file.c, so couldn't be applied to the earlier maint
branch.[2]

These fixes clear all the current C4334 warnings.

The patches can be squashed together if required.

[1] https://lore.kernel.org/git/7fbd4cf4-5f66-a4cd-0c41-e5b12d14d761@iee.email/
[2] https://lore.kernel.org/git/3e7af5d3-58fd-3a92-371f-3fa26cfe05a0@iee.email/

Philip Oakley (4):
  mergesort.c: LLP64 compatibility, upcast unity for left shift
  repack.c: LLP64 compatibility, upcast unity for left shift
  diffcore-delta.c: LLP64 compatibility, upcast unity for left shift
  object-file.c: LLP64 compatibility, upcast unity for left shift

 builtin/repack.c | 2 +-
 diffcore-delta.c | 6 +++---
 mergesort.c      | 2 +-
 object-file.c    | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

Comments

Derrick Stolee Nov. 29, 2021, 2:44 p.m. UTC | #1
On 11/26/2021 6:36 AM, Philip Oakley wrote:
> The Visual Studio MSVC compilation reports a number of C4334 "was 64-bit
> shift intended" size mismatch warnings. In most of these cases a size_t
> is ANDed (masked) with a bit shift of 1, or 1U. On LLP64 systems the unity
> value is 32 bits, while size_t is 64 bits. 
> 
> The fix is to upcast the unity value to size_t.   
> 
> The first patch has also been reported [1] by René Scharfe as an extra patch
> to the rs/mergesort series. That patch had been on maint.
> 
> The middle two patches are similar changes, though [2/4] is a uintptr_t.
> 
> The final patch is applied to object-file.c, which has recently been
> renamed from sha1-file.c, so couldn't be applied to the earlier maint
> branch.[2]
> 
> These fixes clear all the current C4334 warnings.

Thank you for these changes. They all are obviously correct.

I had one style nitpick that you could take or leave.

> The patches can be squashed together if required.

I'm fine either way.

Thanks,
-Stolee