Skip to content

Commit b656d1c

Browse files
gabimeCopilot
andauthored
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 <[email protected]> * Update tests/CMakeLists.txt Co-authored-by: Copilot <[email protected]> * Updated utc_minutes_offset() impl --------- Co-authored-by: Copilot <[email protected]>
1 parent 2670f47 commit b656d1c

File tree

4 files changed

+175
-27
lines changed

4 files changed

+175
-27
lines changed

include/spdlog/details/os-inl.h

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -243,31 +243,29 @@ SPDLOG_INLINE size_t filesize(FILE *f) {
243243
#pragma warning(pop)
244244
#endif
245245

246-
// Return utc offset in minutes or throw spdlog_ex on failure
247246
#if !defined(SPDLOG_NO_TZ_OFFSET)
248-
SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) {
249247
#ifdef _WIN32
250-
#if _WIN32_WINNT < _WIN32_WINNT_WS08
251-
TIME_ZONE_INFORMATION tzinfo;
252-
auto rv = ::GetTimeZoneInformation(&tzinfo);
253-
#else
254-
DYNAMIC_TIME_ZONE_INFORMATION tzinfo;
255-
auto rv = ::GetDynamicTimeZoneInformation(&tzinfo);
256-
#endif
257-
if (rv == TIME_ZONE_ID_INVALID) throw_spdlog_ex("Failed getting timezone info. ", errno);
248+
// Compare the timestamp as Local (mktime) vs UTC (_mkgmtime) to get the offset.
249+
SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) {
250+
std::tm local_tm = tm; // copy since mktime might adjust it (normalize dates, set tm_isdst)
251+
std::time_t local_time_t = std::mktime(&local_tm);
252+
if (local_time_t == -1) {
253+
return 0; // fallback
254+
}
258255

259-
int offset = -tzinfo.Bias;
260-
if (tm.tm_isdst) {
261-
offset -= tzinfo.DaylightBias;
262-
} else {
263-
offset -= tzinfo.StandardBias;
256+
std::time_t utc_time_t = _mkgmtime(&local_tm);
257+
if (utc_time_t == -1) {
258+
return 0; // fallback
264259
}
265-
return offset;
266-
#else
267-
auto offset_seconds = tm.tm_gmtoff;
260+
auto offset_seconds = utc_time_t - local_time_t;
268261
return static_cast<int>(offset_seconds / 60);
269-
#endif
270262
}
263+
#else
264+
// On unix simply use tm_gmtoff
265+
SPDLOG_INLINE int utc_minutes_offset(const std::tm &tm) {
266+
return static_cast<int>(tm.tm_gmtoff / 60);
267+
}
268+
#endif // _WIN32
271269
#endif // SPDLOG_NO_TZ_OFFSET
272270

273271
// Return current thread id as size_t

tests/CMakeLists.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ set(SPDLOG_UTESTS_SOURCES
5050
test_stopwatch.cpp
5151
test_circular_q.cpp
5252
test_bin_to_hex.cpp
53-
test_ringbuffer.cpp)
53+
test_ringbuffer.cpp
54+
test_timezone.cpp
55+
)
5456

5557
if(NOT SPDLOG_NO_EXCEPTIONS)
5658
list(APPEND SPDLOG_UTESTS_SOURCES test_errors.cpp)

tests/test_pattern_formatter.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#include "includes.h"
22
#include "test_sink.h"
3-
3+
#include <regex>
44
#include <chrono>
55

66
using spdlog::memory_buf_t;
@@ -77,18 +77,20 @@ TEST_CASE("date MM/DD/YY ", "[pattern_formatter]") {
7777
oss.str());
7878
}
7979

80-
TEST_CASE("GMT offset ", "[pattern_formatter]") {
80+
// see test_timezone.cpp for actual UTC offset calculation tests
81+
TEST_CASE("UTC offset", "[pattern_formatter]") {
8182
using namespace std::chrono_literals;
8283
const auto now = std::chrono::system_clock::now();
83-
const auto yesterday = now - 24h;
84+
std::string result =
85+
log_to_str_with_time(now, "Some message", "%z", spdlog::pattern_time_type::local, "\n");
8486

8587
#ifndef SPDLOG_NO_TZ_OFFSET
86-
const std::string expected_result = "+00:00\n";
88+
// Match format: +HH:MM or -HH:MM
89+
std::regex re(R"([+-]\d{2}:[0-5]\d\n)");
90+
REQUIRE(std::regex_match(result, re));
8791
#else
88-
const std::string expected_result = "+??:??\n";
92+
REQUIRE(result == "+??:??\n");
8993
#endif
90-
REQUIRE(log_to_str_with_time(yesterday, "Some message", "%z", spdlog::pattern_time_type::utc,
91-
"\n") == expected_result);
9294
}
9395

9496
TEST_CASE("color range test1", "[pattern_formatter]") {

tests/test_timezone.cpp

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
#ifndef SPDLOG_NO_TZ_OFFSET
2+
3+
#include "includes.h"
4+
#include <ctime>
5+
#include <cstdlib>
6+
#include <cstring>
7+
8+
// Helper to construct a simple std::tm from components
9+
std::tm make_tm(int year, int month, int day, int hour, int minute) {
10+
std::tm t;
11+
std::memset(&t, 0, sizeof(t));
12+
t.tm_year = year - 1900;
13+
t.tm_mon = month - 1;
14+
t.tm_mday = day;
15+
t.tm_hour = hour;
16+
t.tm_min = minute;
17+
t.tm_sec = 0;
18+
t.tm_isdst = -1;
19+
std::mktime(&t);
20+
return t;
21+
}
22+
23+
// Cross-platform RAII Helper to safely set/restore process timezone
24+
class ScopedTZ {
25+
std::string original_tz_;
26+
bool has_original_ = false;
27+
28+
public:
29+
explicit ScopedTZ(const std::string& tz_name) {
30+
// save current TZ
31+
#ifdef _WIN32
32+
char* buf = nullptr;
33+
size_t len = 0;
34+
if (_dupenv_s(&buf, &len, "TZ") == 0 && buf != nullptr) {
35+
original_tz_ = std::string(buf);
36+
has_original_ = true;
37+
free(buf);
38+
}
39+
#else
40+
const char* tz = std::getenv("TZ");
41+
if (tz) {
42+
original_tz_ = tz;
43+
has_original_ = true;
44+
}
45+
#endif
46+
47+
// set new TZ
48+
#ifdef _WIN32
49+
_putenv_s("TZ", tz_name.c_str());
50+
_tzset();
51+
#else
52+
setenv("TZ", tz_name.c_str(), 1);
53+
tzset();
54+
#endif
55+
}
56+
57+
~ScopedTZ() {
58+
// restore original TZ
59+
#ifdef _WIN32
60+
if (has_original_) {
61+
_putenv_s("TZ", original_tz_.c_str());
62+
} else {
63+
_putenv_s("TZ", "");
64+
}
65+
_tzset();
66+
#else
67+
if (has_original_) {
68+
setenv("TZ", original_tz_.c_str(), 1);
69+
} else {
70+
unsetenv("TZ");
71+
}
72+
tzset();
73+
#endif
74+
}
75+
};
76+
77+
using spdlog::details::os::utc_minutes_offset;
78+
79+
TEST_CASE("UTC Offset - Western Hemisphere (USA - Standard Time)", "[timezone][west]") {
80+
// EST5EDT: Eastern Standard Time (UTC-5)
81+
ScopedTZ tz("EST5EDT");
82+
83+
// Jan 15th (Winter)
84+
auto tm = make_tm(2023, 1, 15, 12, 0);
85+
REQUIRE(utc_minutes_offset(tm) == -300);
86+
}
87+
88+
TEST_CASE("UTC Offset - Eastern Hemisphere (Europe/Israel - Standard Time)", "[timezone][east]") {
89+
// IST-2IDT: Israel Standard Time (UTC+2)
90+
ScopedTZ tz("IST-2IDT");
91+
92+
// Jan 15th (Winter)
93+
auto tm = make_tm(2023, 1, 15, 12, 0);
94+
REQUIRE(utc_minutes_offset(tm) == 120);
95+
}
96+
97+
TEST_CASE("UTC Offset - Zero Offset (UTC/GMT)", "[timezone][utc]") {
98+
ScopedTZ tz("GMT0");
99+
100+
// Check Winter
101+
auto tm_winter = make_tm(2023, 1, 15, 12, 0);
102+
REQUIRE(utc_minutes_offset(tm_winter) == 0);
103+
104+
// Check Summer (GMT never shifts, so this should also be 0)
105+
auto tm_summer = make_tm(2023, 7, 15, 12, 0);
106+
REQUIRE(utc_minutes_offset(tm_summer) == 0);
107+
}
108+
109+
TEST_CASE("UTC Offset - Non-Integer Hour Offsets (India)", "[timezone][partial]") {
110+
// IST-5:30: India Standard Time (UTC+5:30)
111+
ScopedTZ tz("IST-5:30");
112+
113+
auto tm = make_tm(2023, 1, 15, 12, 0);
114+
REQUIRE(utc_minutes_offset(tm) == 330);
115+
}
116+
117+
TEST_CASE("UTC Offset - Edge Case: Negative Offset Crossing Midnight", "[timezone][edge]") {
118+
ScopedTZ tz("EST5EDT");
119+
// Late night Dec 31st, 2023
120+
auto tm = make_tm(2023, 12, 31, 23, 59);
121+
REQUIRE(utc_minutes_offset(tm) == -300);
122+
}
123+
124+
TEST_CASE("UTC Offset - Edge Case: Leap Year", "[timezone][edge]") {
125+
ScopedTZ tz("EST5EDT");
126+
// Feb 29, 2024 (Leap Day) - Winter
127+
auto tm = make_tm(2024, 2, 29, 12, 0);
128+
REQUIRE(utc_minutes_offset(tm) == -300);
129+
}
130+
131+
TEST_CASE("UTC Offset - Edge Case: Invalid Date (Pre-Epoch)", "[timezone][edge]") {
132+
#ifdef _WIN32
133+
// Windows mktime returns -1 for dates before 1970.
134+
// We expect the function to safely return 0 (fallback).
135+
auto tm = make_tm(1960, 1, 1, 12, 0);
136+
REQUIRE(utc_minutes_offset(tm) == 0);
137+
#else
138+
// Unix mktime handles pre-1970 dates correctly.
139+
// We expect the actual historical offset (EST was UTC-5 in 1960).
140+
ScopedTZ tz("EST5EDT");
141+
auto tm = make_tm(1960, 1, 1, 12, 0);
142+
REQUIRE(utc_minutes_offset(tm) == -300);
143+
#endif
144+
}
145+
146+
#endif // !SPDLOG_NO_TZ_OFFSET

0 commit comments

Comments
 (0)