Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: use time_t_max for empty datetime #2749

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

MatthewGentoo
Copy link
Contributor

In Mu::parse_date_time, when provided with an empty string, return time_t_max instead of G_MAXINT64. For systems with a 64-bit time_t, there is no difference. With a 32-bit time_t it caused a test to fail:

not ok /utils/date-basic - ERROR:../mu-1.12.4/lib/utils/tests/test-utils.cc:92
void test_date_basic(): assertion failed
(parse_date_time(std::get<0>(test), std::get<1>(test)).value_or(-1)
  == std::get<2>(test)): (18446744073709551615 == 2147483647)

This edge case probably only affected the test, as when other parts of the application call parse_date_time (e.g. mu-server.cc and mu-query-processor.cc), they check if the input string is empty first.

In Mu::parse_date_time, when provided with an empty string, return
time_t_max instead of G_MAXINT64. For systems with a 64-bit time_t, there
is no difference. With a 32-bit time_t it caused a test to fail:

    not ok /utils/date-basic - ERROR:../mu-1.12.4/lib/utils/tests/test-utils.cc:92
    void test_date_basic(): assertion failed
    (parse_date_time(std::get<0>(test), std::get<1>(test)).value_or(-1)
      == std::get<2>(test)): (18446744073709551615 == 2147483647)

This edge case probably only affected the test, as when other parts of
the application call parse_date_time (e.g. mu-server.cc and
mu-query-processor.cc), they check if the input string is empty first.
@djcb djcb merged commit e63110f into djcb:master Aug 26, 2024
2 checks passed
@djcb
Copy link
Owner

djcb commented Aug 26, 2024

Makes sense. Merged, thanks.

@MatthewGentoo MatthewGentoo deleted the utils-datetime-max branch August 27, 2024 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants