Re: [PATCH v5 09/11] pack-objects: shrink size field in struct object_entry

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 18, 2018, 08:23 UTC
Message-ID: <CACsJy8AXSCw1b0FpsUv85kZ_HYhHy9OHeDZGGFFg9pOno8GfNw@mail.gmail.com>
In-Reply-To: <xmqq8taqx949.fsf@gitster-ct.c.googlers.com>

On Sun, Mar 18, 2018 at 6:09 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 8 quoted lines

>> +     uint32_t truncated_limit = (uint32_t)limit;
>> +
>> +     return limit == truncated_limit;
>> +}
>
> I am guessing that a compiler that is clever enough will make this
> function a no-op on a 32-bit arch and that is why it is a static
> inline function?

It's a separate function because I don't want to duplicate this == logic twice. Even if the compiler does not optimize this, it's still much cheaper than oe_sze() which involves disk access.

Show 16 quoted lines

>> +static inline int oe_size_less_than(const struct object_entry *e,
>> +                                 unsigned long limit)
>> +{
>> +     if (e->size_valid)
>> +             return e->size_ < limit;
>
> e->size_ is the true size so we can compare it to see if it is smaller
> than limit.
>
>> +     if (contains_in_32bits(limit))
>> +             return 1;
>
> If limit is small enough, and because e->size_valid means e->size_
> does not fit in 32-bit, we know size is larger than limit.
> Shouldn't we be returning 0 that means "no, the size is not less
> than limit" from here?

Argh!!! This logic keeps messing with my brain.

-- 
Duy