<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Jun 26, 2026 at 7:02 PM Jonathan Wakely <<a href="mailto:jwakely@redhat.com">jwakely@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Although the systemd docs say that /etc/localtime should be a symlink to<br>
one of the zoneinfo files, some systems make it a symlink to another<br>
path, where that second path is a symlink to a zoneinfo file (e.g. if<br>
/etc is mounted read-only then /etc/localtime can be a symlink to<br>
another symlink on a writable disk, so that the system timezone can be<br>
altered by re-pointing the symlink on the writable disk).<br>
<br>
In that case, using readlink would only tell us the location of the<br>
second symlink, not which zoneinfo file it points to. Therefore, we<br>
would not be able to extract a valid time zone name from the path, and<br>
chrono::current_zone() would fail.<br>
<br>
To support multiple symlinks we could recursively keep resolving<br>
symlinks with readlink until we reach a path from which we can extract a<br>
zone name. Alternatively, we can just use realpath to resolve all<br>
symlinks to a physical file, which will be . This means we only need one system call and<br>
don't need the extra complexity of calling readlink in a loop.<br>
<br>
The realpath system call also removes redunant slashes, so we can remove<br>
the code that did that manually.<br>
<br>
The possible downsides of this approach that I'm aware of are:<br>
<br>
- When /etc/localtime is a symlink to /invalid/Europe/London but that<br>
file doesn't exist. With the previous implementation we would have<br>
resolved that symlink to the zone "Europe/London" as long as that name<br>
is known to the current chrono::tzdb object. With this change, we<br>
won't get a valid zone name and current_zone() will fail. I'm not sure<br>
how realistic this case is. It might be plausible if libstdc++ is<br>
using the embedded static copy of tzdata.zi and there are no zoneinfo<br>
files on disk at all. In that case the system might still use<br>
/etc/localtime to name a zone, even though the symlink is dangling.<br>
Maybe we could fall back to filesystem::weakly_canonical for this<br>
case?<br></blockquote><div>I would be concerned about doing that silently, as this could lead to</div><div>non-local errors in the deployed environment are extremely hard to debug.</div><div>What I mean is the situation where tzdata should be mounted. </div><div>If the mounting fails, we will resolve the zone without issue, and then only</div><div>If the data requires updating, the update will fail. From that perspective I much</div><div>more prefer loud failure.</div><div><br></div><div>As the fact that this works is libstdc++ specific (other libraries and clients</div><div>may use the content of the file), I think this case (if appears) may be better</div><div>served by a dedicated enviroment variable.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
- When /etc/localtime is a symlink to /usr/share/zoneinfo/Foo/Bar where<br>
"Foo/Bar" is a valid zone in the chrono::tzdb object, but the Bar file<br>
is another symlink to ./Baz where "Foo/Bar" is also a valid zone.<br>
With the previous implementation current_zone() would have returned<br>
the "Foo/Bar" zone. With this change it would return "Foo/Baz". I<br>
don't think it's realistic to have two zones which are distinct zones<br>
(not a Zone and a Link to it)</blockquote>Yes, and the link name is not accessible to the program: zone->name</div><div class="gmail_quote gmail_quote_container">returns the target zone, so it is not observable if we located Asia/Istanbul</div><div class="gmail_quote gmail_quote_container">(that was symlink to Europe/Istanbul) or Asia/Istanbul. I think that is OK.</div><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> but where one of them is defined on-disk<br>
using a symlink to the other.<br>
<br>
libstdc++-v3/ChangeLog:<br>
<br>
PR libstdc++/125467<br></blockquote><div>This PR is created by you. Does it come from real user complain, or this</div><div>was found by you?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
* src/c++20/tzdb.cc (tzdb::current_zone): Use realpath to<br>
resolve the /etc/localtime symlink instead of readlink.<br>
---<br>
<br>
Tested x86_64-linux.<br>
<br>
Should we handle the dangling symlink case, maybe by using<br>
filesystem::weakly_canonical?</blockquote><div>I would wait to see if anybody is actually impacted by it in non-error case.</div><div>I would got with this with only small modification suggested bellow.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
libstdc++-v3/src/c++20/tzdb.cc | 65 +++++++++++-----------------------<br>
1 file changed, 20 insertions(+), 45 deletions(-)<br>
<br>
diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc<br>
index 0158659f79e1..acfc57f76437 100644<br>
--- a/libstdc++-v3/src/c++20/tzdb.cc<br>
+++ b/libstdc++-v3/src/c++20/tzdb.cc<br>
@@ -41,8 +41,13 @@<br>
# include <ext/concurrence.h> // __gnu_cxx::__mutex<br>
#endif<br>
<br>
-#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H)<br>
-# include <unistd.h> // readlink<br>
+#ifdef _GLIBCXX_HAVE_UNISTD_H<br>
+# include <unistd.h> // _XOPEN_VERSION<br>
+#endif<br>
+#if defined _GLIBCXX_USE_REALPATH && _XOPEN_VERSION >= 700<br>
+# include <stdlib.h> // malloc, free, realpath<br>
+#else<br>
+# include <filesystem> // filesystem::canonicalize<br>
#endif<br>
<br>
#ifdef _AIX<br>
@@ -2094,58 +2099,28 @@ constinit tzdb_list::_Node::NumLeapSeconds tzdb_list::_Node::num_leap_seconds;<br>
// to have a way to force a re-read.<br>
<br>
#if !defined(_AIX) && !defined(_GLIBCXX_HAVE_WINDOWS_H)<br>
-#if defined(_GLIBCXX_HAVE_READLINK) && defined(_GLIBCXX_HAVE_UNISTD_H)<br>
- string_view str;<br>
- char buf[128]; // strlen("../usr/share/zoneinfo/...") is usually < 55<br>
- string dynbuf;<br>
// /etc/localtime should be a symlink that ends with a zone name,<br>
// e.g. /etc/localtime -> /usr/share/zoneinfo/Europe/London<br>
// <a href="https://www.freedesktop.org/software/systemd/man/latest/localtime.html" rel="noreferrer" target="_blank">https://www.freedesktop.org/software/systemd/man/latest/localtime.html</a><br>
// This should work on GNU/Linux, macOS, NetBSD, and OpenBSD.<br>
// Some FreeBSD systems also use a symlink for /etc/localtime.<br>
- // Use readlink directly to avoid std::filesystem overhead.<br>
- if (auto n = ::readlink("/etc/localtime", buf, sizeof(buf)); n > 0)<br>
+<br>
+#if defined _GLIBCXX_USE_REALPATH && _XOPEN_VERSION >= 700<br>
+ unique_ptr<char[], void(*)(void*)> buf{ nullptr, &::free };<br>
+ string_view str;<br></blockquote><div>A slight stylistic note: I would prefer `str` to always be string_view (substr will do</div><div>different thing otherwise, that may bite us), and declared before the #if.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ // Use realpath directly to avoid std::filesystem overhead.<br>
+ // We use realpath not readlink to resolve multiple levels of symlinks.<br>
+ if (char* p = ::realpath("/etc/localtime", nullptr))<br>
{<br>
- if (static_cast<size_t>(n) < sizeof(buf))<br>
- str = string_view(buf, n);<br>
- else [[unlikely]]<br>
- {<br>
- // We read the symlink but it didn't fit in buf[], use dynbuf.<br>
- do<br>
- {<br>
- n *= 2;<br>
- dynbuf.__resize_and_overwrite(n, [](char* p, size_t len) {<br>
- auto n2 = ::readlink("/etc/localtime", p, len);<br>
- if (n2 == -1) // symlink removed or replaced by file?!<br>
- __throw_runtime_error("tzdb: error reading /etc/localtime");<br>
- const size_t r = n2;<br>
- return r < len ? r : 0;<br>
- });<br>
- }<br>
- while (dynbuf.empty());<br>
- str = dynbuf;<br>
- }<br>
+ buf.reset(p);<br>
+ str = p;<br>
}<br>
+#else<br>
+ string str = std::filesystem::canonical("/etc/localtime").string();<br></blockquote><div>And here we would have:</div><div> string buf = std::filesystem::canonical("/etc/localtime").string();</div><div> str = buf;</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+#endif<br>
<br>
if (!str.empty())<br>
{<br>
- // Remove any redundant slashes so we can match zone names.<br>
- // e.g. /usr/share/zoneinfo/Europe//London is a valid symlink,<br>
- // but won't match against "Europe/London".<br>
- if (auto pos = str.rfind("//"); pos != str.npos) [[unlikely]]<br>
- {<br>
- if (str.data() != dynbuf.data())<br>
- dynbuf = str;<br>
- string::size_type spos = pos;<br>
- do<br>
- {<br>
- dynbuf.erase(spos, 1);<br>
- spos = dynbuf.rfind("//", spos);<br>
- }<br>
- while (spos != dynbuf.npos);<br>
- str = dynbuf;<br>
- }<br>
-<br>
// Check the trailing components of the path against known zone names.<br>
// Valid IANA times zones can have one, two, or three parts, e.g.<br>
// "UTC", "Europe/London", and "America/Indiana/Indianapolis".<br>
@@ -2171,7 +2146,7 @@ constinit tzdb_list::_Node::NumLeapSeconds tzdb_list::_Node::num_leap_seconds;<br>
str.substr(pos + 1)))<br>
return tz;<br>
}<br>
-#endif<br>
+<br>
// Otherwise, look for a file naming the time zone.<br>
string_view files[] {<br>
"/etc/timezone", // Debian derivates<br>
-- <br>
2.54.0<br>
<br>
</blockquote></div></div>