On Wed, Jul 1, 2026 at 9:30 AM Jonathan Wakely <[email protected]> wrote:
> > > On Wed, 1 Jul 2026, 07:45 Tomasz Kaminski, <[email protected]> wrote: > >> >> >> On Tue, Jun 30, 2026 at 7:48 PM Jonathan Wakely <[email protected]> >> wrote: >> >>> Although the systemd docs say that /etc/localtime should be a symlink to >>> one of the zoneinfo files, some systems make it a symlink to another >>> path, where that second path is a symlink to a zoneinfo file (e.g. if >>> /etc is mounted read-only then /etc/localtime can be a symlink to >>> another symlink on a writable disk, so that the system timezone can be >>> altered by re-pointing the symlink on the writable disk). >>> >>> In that case, using readlink would only tell us the location of the >>> second symlink, not which zoneinfo file it points to. Therefore, we >>> would not be able to extract a valid time zone name from the path, and >>> chrono::current_zone() would fail. >>> >>> To support multiple symlinks we could recursively keep resolving >>> symlinks with readlink until we reach a path from which we can extract a >>> zone name. Alternatively, we can just use realpath to resolve all >>> symlinks to a physical file (which is what HowardHinnant/date does). >>> This means we only need one system call and don't need the extra >>> complexity of calling readlink in a loop. >>> >>> The realpath system call also removes redunant slashes, so we can remove >>> the code that did that manually. >>> >>> The possible downsides of this approach that I'm aware of are: >>> >>> - When /etc/localtime is a symlink to /invalid/Europe/London but that >>> file doesn't exist. With the previous implementation we would have >>> resolved that symlink to the zone "Europe/London" as long as that name >>> is known to the current chrono::tzdb object. With this change, we >>> won't get a valid zone name and current_zone() will fail. I'm not sure >>> how realistic this case is. It might be plausible if libstdc++ is >>> using the embedded static copy of tzdata.zi and there are no zoneinfo >>> files on disk at all. In that case the system might still use >>> /etc/localtime to name a zone, even though the symlink is dangling. >>> We could fall back to filesystem::weakly_canonical for this case, but >>> this patch leaves that for a future change, if it turns out to be >>> needed by any users. >>> >>> - When /etc/localtime is a symlink to /usr/share/zoneinfo/Foo/Bar where >>> "Foo/Bar" is a valid zone in the chrono::tzdb object, but the Bar file >>> is another symlink to ./Baz where "Foo/Bar" is also a valid zone. >>> With the previous implementation current_zone() would have returned >>> the "Foo/Bar" zone. With this change it would return "Foo/Baz". I >>> don't think it's realistic to have two zones which are distinct zones >>> (not a Zone and a Link to it) but where one of them is defined on-disk >>> using a symlink to the other. >>> >>> libstdc++-v3/ChangeLog: >>> >>> PR libstdc++/125467 >>> * src/c++20/tzdb.cc (tzdb::current_zone): Use realpath to >>> resolve the /etc/localtime symlink instead of readlink. >>> --- >>> >>> v2: Make the type of 'str' always std::string_view. Check str != >>> "/etc/localtime" so that we don't bother trying to extract a zone name >>> from the symlink target if it isn't even a symlink. >>> >>> Tested x86_64-linux. >>> >> LGTM with very small suggestion. >> >>> >>> libstdc++-v3/src/c++20/tzdb.cc | 76 +++++++++++++--------------------- >>> 1 file changed, 28 insertions(+), 48 deletions(-) >>> >>> diff --git a/libstdc++-v3/src/c++20/tzdb.cc >>> b/libstdc++-v3/src/c++20/tzdb.cc >>> index 9e601fc176f3..c658e0c9cd37 100644 >>> --- a/libstdc++-v3/src/c++20/tzdb.cc >>> +++ b/libstdc++-v3/src/c++20/tzdb.cc >>> @@ -41,8 +41,13 @@ >>> # include <ext/concurrence.h> // __gnu_cxx::__mutex >>> #endif >>> >>> -#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H) >>> -# include <unistd.h> // readlink >>> +#ifdef _GLIBCXX_HAVE_UNISTD_H >>> +# include <unistd.h> // _XOPEN_VERSION >>> +#endif >>> +#if defined _GLIBCXX_USE_REALPATH && _XOPEN_VERSION >= 700 >>> +# include <stdlib.h> // malloc, free, realpath >>> +#else >>> +# include <filesystem> // filesystem::canonicalize >>> #endif >>> >>> #ifdef _AIX >>> @@ -2098,58 +2103,33 @@ constinit tzdb_list::_Node::NumLeapSeconds >>> tzdb_list::_Node::num_leap_seconds; >>> // to have a way to force a re-read. >>> >>> #if !defined(_AIX) && !defined(_GLIBCXX_HAVE_WINDOWS_H) >>> -#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H) >>> - string_view str; >>> - char buf[128]; // strlen("../usr/share/zoneinfo/...") is usually < >>> 55 >>> - string dynbuf; >>> // /etc/localtime should be a symlink that ends with a zone name, >>> // e.g. /etc/localtime -> /usr/share/zoneinfo/Europe/London >>> // >>> https://www.freedesktop.org/software/systemd/man/latest/localtime.html >>> // This should work on GNU/Linux, macOS, NetBSD, and OpenBSD. >>> - // Some FreeBSD systems also use a symlink for /etc/localtime. >>> - // Use readlink directly to avoid std::filesystem overhead. >>> - if (auto n = ::readlink("/etc/localtime", buf, sizeof(buf)); n > 0) >>> + // Some FreeBSD systems also use a symlink for /etc/localtime >>> (since 15.0). >>> + >>> + // N.B. we do not support dangling symlinks here. If that becomes >>> necessary >>> + // then after realpath fails we could fallback to using >>> + // >>> filesystem::weakly_canonical(filesystem::read_symlink("etc/localtime")). >>> + >>> +#if defined _GLIBCXX_USE_REALPATH && _XOPEN_VERSION >= 700 >>> + unique_ptr<char[], void(*)(void*)> cbuf{ nullptr, &::free }; >>> + string_view str; >>> >> This is very subjective and not necessary, but I would prefer this >> string_view to be >> declared before #if here. I find it easier to follow that way. >> > > The order I used means that the string view lifetime is shorter than the > buffer lifetime so it can't dangle. > OK. You would need to observe that from the destructor of the object created before this #ifdef, but it is some argument. > > > >> >>> + // Use realpath directly to avoid std::filesystem overhead. >>> + // We use realpath not readlink to resolve multiple levels of >>> symlinks. >>> + if (char* p = ::realpath("/etc/localtime", nullptr)) >>> { >>> - if (static_cast<size_t>(n) < sizeof(buf)) >>> - str = string_view(buf, n); >>> - else [[unlikely]] >>> - { >>> - // We read the symlink but it didn't fit in buf[], use >>> dynbuf. >>> - do >>> - { >>> - n *= 2; >>> - dynbuf.__resize_and_overwrite(n, [](char* p, size_t len) >>> { >>> - auto n2 = ::readlink("/etc/localtime", p, len); >>> - if (n2 == -1) // symlink removed or replaced by file?! >>> - __throw_runtime_error("tzdb: error reading >>> /etc/localtime"); >>> - const size_t r = n2; >>> - return r < len ? r : 0; >>> - }); >>> - } >>> - while (dynbuf.empty()); >>> - str = dynbuf; >>> - } >>> + cbuf.reset(p); >>> + str = p; >>> } >>> +#else >>> + string sbuf = std::filesystem::canonical("/etc/localtime").string(); >>> + string_view str = sbuf; >>> >> And this will become an assignment. >> >>> +#endif >>> >>> - if (!str.empty()) >>> + if (!str.empty() && str != "/etc/localtime") >>> { >>> - // Remove any redundant slashes so we can match zone names. >>> - // e.g. /usr/share/zoneinfo/Europe//London is a valid symlink, >>> - // but won't match against "Europe/London". >>> - if (auto pos = str.rfind("//"); pos != str.npos) [[unlikely]] >>> - { >>> - if (str.data() != dynbuf.data()) >>> - dynbuf = str; >>> - string::size_type spos = pos; >>> - do >>> - { >>> - dynbuf.erase(spos, 1); >>> - spos = dynbuf.rfind("//", spos); >>> - } >>> - while (spos != dynbuf.npos); >>> - str = dynbuf; >>> - } >>> - >>> // Check the trailing components of the path against known zone >>> names. >>> // Valid IANA times zones can have one, two, or three parts, e.g. >>> // "UTC", "Europe/London", and "America/Indiana/Indianapolis". >>> @@ -2175,10 +2155,10 @@ constinit tzdb_list::_Node::NumLeapSeconds >>> tzdb_list::_Node::num_leap_seconds; >>> str.substr(pos + 1))) >>> return tz; >>> } >>> -#endif >>> + >>> // Otherwise, look for a file naming the time zone. >>> string_view files[] { >>> - "/etc/timezone", // Debian derivates >>> + "/etc/timezone", // Debian derivates, non-systemd Gentoo >>> "/var/db/zoneinfo", // FreeBSD >>> }; >>> for (auto f : files) >>> -- >>> 2.54.0 >>> >>>
