Skip to content

parsedate: refactor and 64-bit math adjustments#21394

Closed
bagder wants to merge 8 commits into
masterfrom
bagder/parsedate-refactor
Closed

parsedate: refactor and 64-bit math adjustments#21394
bagder wants to merge 8 commits into
masterfrom
bagder/parsedate-refactor

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 21, 2026

No description provided.

bagder added 2 commits April 21, 2026 12:43
- introduce 'struct when' to hold the parser result
- initwhen() initializes a 'struct when'
- datestring() parses strings
- datenum() parses numbers
- datecheck() does some final checks
- tzadjust() adds the time zone offset
- convert math to 64 bit, squeeze into time_t only in the last step,
  mktimet() does the time_t storing
@github-actions github-actions Bot added the tests label Apr 21, 2026
@bagder bagder marked this pull request as ready for review April 21, 2026 10:51
@bagder bagder requested a review from Copilot April 21, 2026 10:51
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the date parsing implementation in lib/parsedate.c to use a structured “when” representation and to route calculations through curl_off_t, aiming to improve clarity and handling of wider time ranges.

Changes:

  • Introduces struct when and splits parsing into helper functions (datestring, datenum, datecheck, tzadjust, mktimet).
  • Changes the epoch calculation path to operate on curl_off_t seconds before converting to time_t.
  • Updates tests/libtest/lib517.c to gate the post-Y2038 second on 64-bit time_t.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/libtest/lib517.c Adjusts Y2038-related test gating for 64-bit time_t.
lib/parsedate.c Refactors parsing and time math to use struct when + curl_off_t intermediate seconds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/parsedate.c
Comment thread lib/parsedate.c Outdated
Comment thread lib/parsedate.c
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 21, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 21, 2026

🤖 Augment PR Summary

Summary: Refactors parsedate to centralize parsed fields and make epoch conversion use wider intermediate math.

Changes:

  • Introduces a `struct when` to hold all parsed date/time components and timezone offset.
  • Splits parsing into helpers (`initwhen`, `datestring`, `datenum`, `datecheck`) to simplify control flow.
  • Changes GMT epoch conversion to return `curl_off_t` and adds `mktimet()` to clamp/cast into `time_t`.
  • Adds `tzadjust()` for applying timezone offsets with overflow clamping at the `curl_off_t` stage.
  • Removes coarse year-based 32-bit `time_t` bounds and instead clamps based on computed seconds, allowing valid early-2038 timestamps.
  • Extends handling for unsigned `time_t` to reject negative second values.
  • Updates `lib517` coverage so 2038-01-19 03:14:07 is always tested, and the next-second case remains 64-bit-only.

Technical Notes: The parser now treats non-alphanumeric characters as separators, and performs final range decisions in mktimet() to better match platform time_t capabilities.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/parsedate.c
@bagder bagder closed this in 2e36070 Apr 21, 2026
@bagder bagder deleted the bagder/parsedate-refactor branch April 21, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants