Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 14, 2016, 20:22 UTC
Message-ID: <xmqqbn20otqu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160714201018.GA18372@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

>> +#if ULONG_MAX == 0x7FFFFFFF
>> +#define USTAR_MAX_SIZE ULONG_MAX
>> +#define USTAR_MAX_MTIME ULONG_MAX
>> +#else
>>  #define USTAR_MAX_SIZE 077777777777UL
>>  #define USTAR_MAX_MTIME 077777777777UL
>> +#endif
>>  
>>  /* writes out the whole block, but only if it is full */
>>  static void write_if_needed(void)
>
> If for some reason we are wrong that objects cannot be larger than
> ULONG_MAX (e.g., later on we convert everything to size_t, and 64-bit
> LLP platforms handle large objects just fine), then we would prematurely
> switch to extended headers on those platforms.
>
> I think that's OK. This would just need cleaned up as part of the
> conversion from "unsigned long" to "size_t" (the correct check would
> then be against the max size_t).

> Also, shouldn't it be checking against 0xFFFFFFFF?

Correct.  Somehow I thought I was checking with LONG_MAX.  Will correct.

> An easier check would be "sizeof()", but I guess we can't use that in a
> preprocessor directive.

Yes, I tried it and failed ;-)

Show 9 quoted lines

>> -test_expect_success TAR_HUGE 'system tar can read our huge size' '
>> +test_expect_success TAR_HUGE,64BIT 'system tar can read our huge size' '
>
> The 64BIT prereq is really "unsigned long is 64-bit". I wonder if we
> should call it UL64 or something like that to make it more clear.
>
> That makes it unnecessarily tied-in with the implementation, but it does
> make it more clear what we care about; the distinction matters for
> things like LP64 vs LLP64.

I do not think any platform is weird enough to have different sizes for long and ulong, so I am not sure you need UL64.

But pointer size can legitimately be different, so it has a value to differentiate between LP64 and LLP64, if we start doing things like "does this platform have large virtual address space?"

LONG_IS_64BIT perhaps to be more readable?