Re: [PATCH v7] diff-delta: use safe integer arithmetic

[Junio C Hamano <gitster@pobox.com>]

"Pablo Sabater" <pabloosabaterr@gmail.com> writes:

> On Tue Oct 27, 2026 at 11:00 AM WET, Srujith A Poojary via GitGitGadget wrote:
>> From: Srujith A Poojary <poojarysrujith@gmail.com>
>>
> [snip]
>>
>> For the unpacked index allocation:
>> - sizeof(struct unpacked_index_entry) is exactly 16 bytes on 32-bit systems

"exactly 16 bytes"?  That sounds fishy.

It is implementation defined, a naive expectation is it would be
more common to make it occupy 12 bytes, and we should expect it to
be larger like 16 bytes depending on platforms.

The C standard (e.g., ISO/IEC 9899:1999) has this:

    Within a structure object, the non-bit-field members and the
    units in which bit-fields reside have addresses that increase in
    the order in which they are declared.  A pointer to a structure
    object, suitably converted, points to its initial member (or if
    that member is a bit-field, then to the unit in which it
    resides), and vice versa.  There may be unnamed padding within a
    structure object, but not at its beginning.

For a 32-bit platform, the minimum natural alignment for a 4-byte
unsigned int, unsigned char *, and a pointer to a struct, are all 4
bytes.  However, the standard specifies minimum requirements, not
maximums.  A compiler is entirely free to choose a stricter
alignment (such as 16 bytes) for performance optimization reasons,
e.g., to align for better cache line utilization or vectorization.

I think your math is correct in that the computation for allocation
size would not overflow if we can assume the struct involved is
always 12-byte on 32-bit systems.  That would make this patch
unnecessary (and the patch is unnecessary on 64-bit systems with
wider size_t to begin with).  But we cannot assume the best case.

We could assume sizeof(struct unpacked_index_entry) == 12 on
platforms where we can depend on GCC/Clang's __attribute__((packed))
and C11's <stdalign.h>, but we are not doing that (and I am not sure
if we want to, as these can introduce different portability issues).

Thanks for pointing out the fishy description.