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

SunOS/Solaris fallback in utc_minutes_offset returns meaningless wrong number of minutes #3351

Open
toh-ableton opened this issue Feb 28, 2025 · 1 comment

Comments

@toh-ableton
Copy link

spdlog uses the function utc_minutes_offset in the z_formatter to format the '%z' part of a format string (UTC-offset) when logging time. The function has three implementations, one of which is chosen at compile time via marcro defines (Windows, SunOS/Solaris, default).

The SunOS/Solaris implementation, however, returns the difference of the given tm and gmtime(now) instead, which can be an arbitrary value. If a unit test for a logging subsystem logs a message at a defined point in time to get identical output in every test run, for instance, the difference could amount to years.

The function returns the correct value only in case tm is at most the fractional part of a second before localtime(now) or roughly a minute after, otherwise the truncation in the return statement will produce wrong values.

auto offset_seconds = helper::calculate_gmt_offset(tm);

A better solution could look like this:

    const auto now = std::time(nullptr);
    const auto local_now = localtime(now);
    auto offset_seconds = helper::calculate_gmt_offset(local_now, gmtime(now));
    if (tm.tm_isdst >= 0 && local_now.tm_isdst >=0) {
        offset_seconds += (tm.tm_isdst > 0) ? 3600 : 0;
        offset_seconds -= (local_now.tm_isdst > 0) ? 3600 : 0;
    }

This assumes that tm has the same timezone as the local time, which seems like the only reasonable assumption on platforms missing the tm_gmtoff-field.

It also assumes that daylight savings time advances the clock by 1h, which is not universally true, e.g. in parts of Australia.

@tt4g
Copy link
Contributor

tt4g commented Feb 28, 2025

PR is welcome.

toh-ableton added a commit to toh-ableton/spdlog that referenced this issue Mar 6, 2025
toh-ableton added a commit to toh-ableton/spdlog that referenced this issue Mar 6, 2025
…workaround)

Apple platforms have had the tm_gmtoff-field at least since Mac OS X 10.0,
as are POSIX.1-2024 conforming systems, which are also required to support
it.

This has the unfortunate effect to use the SunOS/Solaris fallback, which
doesn't compute the correct value if the passed value of tm isn't the
current system time, i.e. localtime(::time()) (gabime#3351).
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

No branches or pull requests

2 participants