Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> "Jonathan Tan via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>> diff --git a/pack-objects.h b/pack-objects.h
>> index b9898a4e64b..15be8368d21 100644
>> --- a/pack-objects.h
>> +++ b/pack-objects.h
>> @@ -207,6 +207,34 @@ static inline uint32_t pack_name_hash(const char *name)
>> return hash;
>> }
>>
>> +static inline uint32_t pack_name_hash_v2(const char *name)
>> +{
>> + uint32_t hash = 0, base = 0, c;
>> + ...
>> + while ((c = *name++)) {
>> + ...
>> + /*
>> + * 'c' is only a single byte. Reverse it and move
>> + * it to the top of the hash, moving the rest to
>> + * less-significant bits.
>> + */
>> + c = (c & 0xF0) >> 4 | (c & 0x0F) << 4;
>> + ...
> This works because `c` is masked before any arithmetic is performed on
> it, but the cast from potentially signed char to uint32_t still makes
> me nervous - if char is signed, it behaves as if it was first cast to
> int32_t and only then to uint32_t, ...
> I would declare `c` as uint8_t or unsigned char.
I think you meant the type of "name", and your worry is that *name
may pick up a negative integer from there when the platform char is
signed? Here we are dealing with a pathname that has often UTF-8
encoded non-ASCII letters with 8th bit set, and I agree with you
that being explicitly unsigned would certainly help reduce the
cognitive load.
Thanks.
