[PATCH v5 0/4] libuuid: Fix pre-1970 UUID timestamp overflow

[Kiran Rangoon <kiranrangoon0@gmail.com>]

> On 2025-12-18 16:31:21-0500, Kiran Rangoon wrote:
> > I revised the code in response to your feedback.

> Thanks!

> > > Looking at this more closely: __uuid_time() is directly exposed to users
> > > of libuuid as uuid_time(). This means that any change of the function's
> > > contract would break all external users and should be avoided at all
> > > costs.
> >
> > Reversed the change there.
> 
> Your new revision is based on top of your old series.
> Instead it should always be based on upstream code.
> In this case the util-linux master branch.

Fixed in v4: I rebased directly on origin/master instead of building
on my previous patch series.
>
> > > Fortunately it turns out that the timestamps embedded in UUIDs
> > > only use 60 bits. This means that the calculation can be performed in an
> > > int64_t without any risk of over- or underflow. Please try to implement
> > > it that way instead.
> >
> > I'm using signed int64_t here as suggested.
> >
> > > It might be useful to change the signature of gregorian_to_unix() to
> > > "static void gregorian_to_unix(uint64_t ts, struct timeval *tv)".
> >
> > I revised the code to use a struct timeval now.
>
> That looks good. However that refactoring should be in its own commit.
> More on that below.

Fixed in v4: The refactoring is now in its own dedicated commit (patch 2/4).
>
> > > Also please perform each logical step in a dedicated commit.
> >
> > > I made two commits, one for the code change and the other for tests, but
> > > if there is a way you would perfer me to do it I could change it.
> >
> The test is a bad candidate to split out. After each commit the
> repository needs to be in a consistent state, which means that the code
> builds and the tests pass. As the code change breaks the existing test,
> the test needs to be adapted in lockstep.

Understood. In v4, the test updates are included with the code changes
that require them, maintaining repository consistency at each commit.
>
> I see the following commits:
>
> 1) Switch to the simpler constant definition
> 2) Pass 'struct timeval' to gregorian_to_unix()
> 3) Fix the overflow, adapt the existing test
> 4) Add a hand full of additional testcases.

Implemented exactly as suggested in v4.

> Also this patch now lost its useful commit message which got replaced by
> our discussion. This should not happen. Each commit should explain why
> it is being done.

Fixed in v4: All commits now have proper descriptive messages explaining
the rationale for each change.
>
> > +     const uint64_t offset = 0x01B21DD213814000ULL;
> > +     int64_t t = (int64_t) ts - (int64_t) offset;
>
> Whitespace corruption?

Fixed in v4: Proper tab indentation is used throughout.
---

This v4 patch series addresses all the feedback points above:

Kiran Rangoon (4):
  libuuid: simplify gregorian-to-unix offset calculation
  libuuid: refactor gregorian_to_unix to populate timeval directly
  libuuid: fix timestamp overflow for pre-1970 dates
  tests: correct UUID timestamp test expectations

 libuuid/src/uuid_time.c       | 15 +++++++--------
 tests/expected/uuid/uuidparse |  5 ++++-
 tests/ts/uuid/uuidparse       |  3 +++
 3 files changed, 14 insertions(+), 9 deletions(-)

-- 
2.47.3