From b656d1ceecac83e969faaf8b35f8a5edb964b0e0 Mon Sep 17 00:00:00 2001 From: Gabi Melman Date: Sun, 4 Jan 2026 18:01:55 +0200 Subject: [PATCH] Windows utc_minutes_offset(): Fix historical DST accuracy and improve offset calculation speed (~2.5x) (#3508) * New utc offset impl for windows and unit tests * Update utc_minutes_offset() * Fix warning * Fix warning * Fix timezone tests * Fix timezone tests * Update tests/test_pattern_formatter.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests/CMakeLists.txt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Updated utc_minutes_offset() impl --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- include/spdlog/details/os-inl.h | 38 ++++---- tests/CMakeLists.txt | 4 +- tests/test_pattern_formatter.cpp | 16 ++-- tests/test_timezone.cpp | 146 +++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 28 deletions(-) create mode 100644 tests/test_timezone.cpp diff --git a/include/spdlog/details/os-inl.h b/include/spdlog/details/os-inl.h index 2acded09..92dfe129 100644 --- a/include/spdlog/details/os-inl.h +++ b/include/spdlog/details/os-inl.h @@ -243,31 +243,29 @@ SPDLOG_INLINE size_t filesize(FILE *f) { #pragma warning(pop) #endif -// Return utc offset in minutes or throw spdlog_ex on failure #if !defined(SPDLOG_NO_TZ_OFFSET) -SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) { #ifdef _WIN32 -#if _WIN32_WINNT < _WIN32_WINNT_WS08 - TIME_ZONE_INFORMATION tzinfo; - auto rv = ::GetTimeZoneInformation(&tzinfo); -#else - DYNAMIC_TIME_ZONE_INFORMATION tzinfo; - auto rv = ::GetDynamicTimeZoneInformation(&tzinfo); -#endif - if (rv == TIME_ZONE_ID_INVALID) throw_spdlog_ex("Failed getting timezone info. ", errno); - - int offset = -tzinfo.Bias; - if (tm.tm_isdst) { - offset -= tzinfo.DaylightBias; - } else { - offset -= tzinfo.StandardBias; +// Compare the timestamp as Local (mktime) vs UTC (_mkgmtime) to get the offset. +SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) { + std::tm local_tm = tm; // copy since mktime might adjust it (normalize dates, set tm_isdst) + std::time_t local_time_t = std::mktime(&local_tm); + if (local_time_t == -1) { + return 0; // fallback } - return offset; -#else - auto offset_seconds = tm.tm_gmtoff; + + std::time_t utc_time_t = _mkgmtime(&local_tm); + if (utc_time_t == -1) { + return 0; // fallback + } + auto offset_seconds = utc_time_t - local_time_t; return static_cast(offset_seconds / 60); -#endif } +#else +// On unix simply use tm_gmtoff +SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) { + return static_cast(tm.tm_gmtoff / 60); +} +#endif // _WIN32 #endif // SPDLOG_NO_TZ_OFFSET // Return current thread id as size_t diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5ba2a106..d4952774 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -50,7 +50,9 @@ set(SPDLOG_UTESTS_SOURCES test_stopwatch.cpp test_circular_q.cpp test_bin_to_hex.cpp - test_ringbuffer.cpp) + test_ringbuffer.cpp + test_timezone.cpp +) if(NOT SPDLOG_NO_EXCEPTIONS) list(APPEND SPDLOG_UTESTS_SOURCES test_errors.cpp) diff --git a/tests/test_pattern_formatter.cpp b/tests/test_pattern_formatter.cpp index 39f47e99..cd0df811 100644 --- a/tests/test_pattern_formatter.cpp +++ b/tests/test_pattern_formatter.cpp @@ -1,6 +1,6 @@ #include "includes.h" #include "test_sink.h" - +#include #include using spdlog::memory_buf_t; @@ -77,18 +77,20 @@ TEST_CASE("date MM/DD/YY ", "[pattern_formatter]") { oss.str()); } -TEST_CASE("GMT offset ", "[pattern_formatter]") { +// see test_timezone.cpp for actual UTC offset calculation tests +TEST_CASE("UTC offset", "[pattern_formatter]") { using namespace std::chrono_literals; const auto now = std::chrono::system_clock::now(); - const auto yesterday = now - 24h; + std::string result = + log_to_str_with_time(now, "Some message", "%z", spdlog::pattern_time_type::local, "\n"); #ifndef SPDLOG_NO_TZ_OFFSET - const std::string expected_result = "+00:00\n"; + // Match format: +HH:MM or -HH:MM + std::regex re(R"([+-]\d{2}:[0-5]\d\n)"); + REQUIRE(std::regex_match(result, re)); #else - const std::string expected_result = "+??:??\n"; + REQUIRE(result == "+??:??\n"); #endif - REQUIRE(log_to_str_with_time(yesterday, "Some message", "%z", spdlog::pattern_time_type::utc, - "\n") == expected_result); } TEST_CASE("color range test1", "[pattern_formatter]") { diff --git a/tests/test_timezone.cpp b/tests/test_timezone.cpp new file mode 100644 index 00000000..7bee9c50 --- /dev/null +++ b/tests/test_timezone.cpp @@ -0,0 +1,146 @@ +#ifndef SPDLOG_NO_TZ_OFFSET + +#include "includes.h" +#include +#include +#include + +// Helper to construct a simple std::tm from components +std::tm make_tm(int year, int month, int day, int hour, int minute) { + std::tm t; + std::memset(&t, 0, sizeof(t)); + t.tm_year = year - 1900; + t.tm_mon = month - 1; + t.tm_mday = day; + t.tm_hour = hour; + t.tm_min = minute; + t.tm_sec = 0; + t.tm_isdst = -1; + std::mktime(&t); + return t; +} + +// Cross-platform RAII Helper to safely set/restore process timezone +class ScopedTZ { + std::string original_tz_; + bool has_original_ = false; + +public: + explicit ScopedTZ(const std::string& tz_name) { + // save current TZ +#ifdef _WIN32 + char* buf = nullptr; + size_t len = 0; + if (_dupenv_s(&buf, &len, "TZ") == 0 && buf != nullptr) { + original_tz_ = std::string(buf); + has_original_ = true; + free(buf); + } +#else + const char* tz = std::getenv("TZ"); + if (tz) { + original_tz_ = tz; + has_original_ = true; + } +#endif + + // set new TZ +#ifdef _WIN32 + _putenv_s("TZ", tz_name.c_str()); + _tzset(); +#else + setenv("TZ", tz_name.c_str(), 1); + tzset(); +#endif + } + + ~ScopedTZ() { + // restore original TZ +#ifdef _WIN32 + if (has_original_) { + _putenv_s("TZ", original_tz_.c_str()); + } else { + _putenv_s("TZ", ""); + } + _tzset(); +#else + if (has_original_) { + setenv("TZ", original_tz_.c_str(), 1); + } else { + unsetenv("TZ"); + } + tzset(); +#endif + } +}; + +using spdlog::details::os::utc_minutes_offset; + +TEST_CASE("UTC Offset - Western Hemisphere (USA - Standard Time)", "[timezone][west]") { + // EST5EDT: Eastern Standard Time (UTC-5) + ScopedTZ tz("EST5EDT"); + + // Jan 15th (Winter) + auto tm = make_tm(2023, 1, 15, 12, 0); + REQUIRE(utc_minutes_offset(tm) == -300); +} + +TEST_CASE("UTC Offset - Eastern Hemisphere (Europe/Israel - Standard Time)", "[timezone][east]") { + // IST-2IDT: Israel Standard Time (UTC+2) + ScopedTZ tz("IST-2IDT"); + + // Jan 15th (Winter) + auto tm = make_tm(2023, 1, 15, 12, 0); + REQUIRE(utc_minutes_offset(tm) == 120); +} + +TEST_CASE("UTC Offset - Zero Offset (UTC/GMT)", "[timezone][utc]") { + ScopedTZ tz("GMT0"); + + // Check Winter + auto tm_winter = make_tm(2023, 1, 15, 12, 0); + REQUIRE(utc_minutes_offset(tm_winter) == 0); + + // Check Summer (GMT never shifts, so this should also be 0) + auto tm_summer = make_tm(2023, 7, 15, 12, 0); + REQUIRE(utc_minutes_offset(tm_summer) == 0); +} + +TEST_CASE("UTC Offset - Non-Integer Hour Offsets (India)", "[timezone][partial]") { + // IST-5:30: India Standard Time (UTC+5:30) + ScopedTZ tz("IST-5:30"); + + auto tm = make_tm(2023, 1, 15, 12, 0); + REQUIRE(utc_minutes_offset(tm) == 330); +} + +TEST_CASE("UTC Offset - Edge Case: Negative Offset Crossing Midnight", "[timezone][edge]") { + ScopedTZ tz("EST5EDT"); + // Late night Dec 31st, 2023 + auto tm = make_tm(2023, 12, 31, 23, 59); + REQUIRE(utc_minutes_offset(tm) == -300); +} + +TEST_CASE("UTC Offset - Edge Case: Leap Year", "[timezone][edge]") { + ScopedTZ tz("EST5EDT"); + // Feb 29, 2024 (Leap Day) - Winter + auto tm = make_tm(2024, 2, 29, 12, 0); + REQUIRE(utc_minutes_offset(tm) == -300); +} + +TEST_CASE("UTC Offset - Edge Case: Invalid Date (Pre-Epoch)", "[timezone][edge]") { +#ifdef _WIN32 + // Windows mktime returns -1 for dates before 1970. + // We expect the function to safely return 0 (fallback). + auto tm = make_tm(1960, 1, 1, 12, 0); + REQUIRE(utc_minutes_offset(tm) == 0); +#else + // Unix mktime handles pre-1970 dates correctly. + // We expect the actual historical offset (EST was UTC-5 in 1960). + ScopedTZ tz("EST5EDT"); + auto tm = make_tm(1960, 1, 1, 12, 0); + REQUIRE(utc_minutes_offset(tm) == -300); +#endif +} + +#endif // !SPDLOG_NO_TZ_OFFSET \ No newline at end of file