All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: m@sph3r3.com
Cc: "René Scharfe" <l.s.r@web.de>, git@vger.kernel.org
Subject: Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range
Date: Mon, 12 Oct 2026 08:07:56 -0700	[thread overview]
Message-ID: <xmqqpkxf9ctv.fsf@gitster.g> (raw)
In-Reply-To: <CA+h9NxRQLP=20kfmSm6B2WtkkmHDCfdRDZEawOpzFF1LoghTEA@mail.gmail.com> (m.'s message of "Mon, 12 Oct 2026 08:56:19 -0400")

m@sph3r3.com writes:

> For the i386 failure Junio reported, the beyond-2038 archive test and
> its dependent omission check should use TIME_T_IS_64BIT. TIME_IS_64BIT
> checks Git's timestamp_t, which can be 64-bit while time_t is 32-bit.

;-)

Thanks.  It is an obvious thing, only once it is pointed out X-<.

These two prerequisites sound too similar, and the latter probably
is no longer very relevant in the modern world, but that was what
was used in the test.

TIME_T_IS_64BIT measures how wide sizeof(time_t) is, which makes
sense as a portability test across platforms.

But TIME_IS_64BIT?  It measures how wide sizeof(timestamp_t) is, but
the thing is, it is uintmax_t everywhere as far as "git grep" can see.

        $ git grep -n -e 'typedef .*timestamp_t' :/
        compat/posix.h:267:typedef uintmax_t timestamp_t;
        $ git grep -n -e timestamp_t\; :/\*.h
        compat/posix.h:267:typedef uintmax_t timestamp_t;

So TIME_IS_64BIT can be true even if TIME_T_IS_64BIT is not.  Unless
we are on a platform where uintmax_t is 32-bit wide, TIME_IS_64BIT
would be true.  And the time value from the system that is delibered
to us initially in time_t would be limited by that even if timestamp_t
is wider.  TIME_IS_64BIT would not have much value as TIME_T_IS_64BIT
as a portability guard to tests that require wider time values across
the callstack.

Perhaps we should audit places where timestamp_t value is read from
SCM data and coerced into time_t to expose it in a form that outside
world (like ZIP archive format) would understand and warn when we
truncate values?  I will leave it as #leftoverbits for others to
think about it through ;-).

Thanks.

      parent reply	other threads:[~2026-10-12 15:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-05 13:48 [BUG] ZIP timestamp conversion and strict fast-import date validation Matthew E. Luallen
2026-10-05 19:49 ` René Scharfe
2026-10-05 20:32   ` Matthew E. Luallen
2026-10-07 15:33     ` René Scharfe
2026-10-06  3:22   ` m
2026-10-11 16:06 ` [PATCH] archive-zip: omit UT extra field if mtime is out of range René Scharfe
2026-10-12  2:08   ` Junio C Hamano
2026-10-12 14:39     ` René Scharfe
2026-10-12 12:56   ` m
2026-10-12 14:45     ` René Scharfe
2026-10-12 15:07     ` Junio C Hamano [this message]

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=xmqqpkxf9ctv.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=m@sph3r3.com \
    /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.