All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>,
	"Johannes Sixt" <j6t@kdbg.org>
Subject: Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers
Date: Thu, 14 Jul 2016 13:22:01 -0700	[thread overview]
Message-ID: <xmqqbn20otqu.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160714201018.GA18372@sigill.intra.peff.net> (Jeff King's message of "Thu, 14 Jul 2016 16:10:18 -0400")

Jeff King <peff@peff.net> writes:

>> +#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 ;-)

>> -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?

  reply	other threads:[~2016-07-14 20:22 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30  9:06 [PATCH v4 0/5] friendlier handling of overflows in archive-tar Jeff King
2016-06-30  9:07 ` [PATCH v4 1/5] t9300: factor out portable "head -c" replacement Jeff King
2016-07-01  4:45   ` Eric Sunshine
2016-07-01 17:23   ` Junio C Hamano
2016-07-01 18:01     ` Jeff King
2016-06-30  9:08 ` [PATCH v4 2/5] t5000: test tar files that overflow ustar headers Jeff King
2016-07-14 15:47   ` Johannes Schindelin
2016-07-14 16:45     ` Johannes Sixt
2016-07-14 17:08       ` Junio C Hamano
2016-07-14 20:52         ` Johannes Sixt
2016-07-14 21:32           ` Jeff King
2016-07-14 22:30             ` Junio C Hamano
2016-07-14 22:38               ` Jeff King
2016-07-15 13:37                 ` Torsten Bögershausen
2016-07-15 13:46                   ` Jeff King
2016-07-14 22:26           ` Junio C Hamano
2016-07-14 18:24       ` Jeff King
2016-07-14 18:21     ` Jeff King
2016-07-14 20:00       ` Junio C Hamano
2016-07-14 20:03         ` Junio C Hamano
2016-07-14 20:14           ` Jeff King
2016-07-14 20:09         ` Junio C Hamano
2016-07-14 20:10         ` Jeff King
2016-07-14 20:22           ` Junio C Hamano [this message]
2016-07-14 20:27             ` Jeff King
2016-07-14 20:34               ` Junio C Hamano
2016-07-14 20:43                 ` [PATCH v2 0/2] ulong may only be 32-bit wide Junio C Hamano
2016-07-14 20:43                   ` [PATCH v2 1/2] t0006: skip "far in the future" test when unsigned long is not long enough Junio C Hamano
2016-07-14 20:43                   ` [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit Junio C Hamano
2016-07-14 22:20                     ` Jeff King
2016-07-14 22:36                       ` Junio C Hamano
2016-07-16  6:28                         ` Duy Nguyen
2016-07-15 15:10                 ` [PATCH v4 2/5] t5000: test tar files that overflow ustar headers Johannes Schindelin
2016-07-15 16:49                   ` Junio C Hamano
2016-06-30  9:09 ` [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB Jeff King
2016-07-14 16:48   ` Johannes Sixt
2016-07-14 17:11     ` Junio C Hamano
2016-07-14 18:16       ` Jeff King
2016-07-15  2:59     ` Torsten Bögershausen
2016-06-30  9:09 ` [PATCH v4 4/5] archive-tar: write extended headers for far-future mtime Jeff King
2016-06-30  9:09 ` [PATCH v4 5/5] archive-tar: drop return value Jeff King
2016-06-30  9:14 ` [PATCH v4 6/5] t5000: use test_match_signal Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbn20otqu.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.