Skip to content

8255239: The timezone of the hs_err_pid log file is corrupted in Japanese locale #1023

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.

Sign up for GitHub

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

Closed
calvinccheung wants to merge 1 commit into from
Closed

8255239: The timezone of the hs_err_pid log file is corrupted in Japanese locale #1023

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/hotspot/share/runtime/os.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,8 +1002,14 @@ void os::print_date_and_time(outputStream *st, char* buf, size_t buflen) {

struct tm tz;
if (localtime_pd(&tloc, &tz) != NULL) {
::strftime(buf, buflen, "%Z", &tz);
st->print("Time: %s %s", timestring, buf);
wchar_t w_buf[80];
size_t n = ::wcsftime(w_buf, 80, L"%Z", &tz);
Copy link
Member

@iklam iklam Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://linux.die.net/man/3/wcsftime: "The Linux implementation of this interface may differ (consult the corresponding Linux manual page for details of Linux behavior), or the interface may not be implemented on Linux."

Also, the L"%Z" notation is Windows-specific.

Maybe we should use the new code only on Windows?

An alternative is to use the C++ standard library (std::wcsftime and std:: wcstombs). However, this part of std:: is not yet permitted -- see https://bugs.openjdk.java.net/browse/JDK-8208089

Copy link
Member

@iklam iklam Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After further investigation -- wcsftime is in C99. Also, we use it here only inside if (localtime_pd(&tloc, &tz) != NULL). I supposed any Linux distro that has a minimal of locale support to make that function return non-null would have a working implementation of wcsftime.

So I think this code is OK. The only change I request is to change L"%Z" to "%Z"

Copy link
Member

@iklam iklam Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon even more investigation, I was completely wrong :-)

According https://en.cppreference.com/w/cpp/language/string_literal, the "L" prefix is for wchar_t string literals. So your code is correct.

Copy link
Member Author

@calvinccheung calvinccheung Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iklam Thanks for your review and making sure the change is ok.

if (n > 0) {
::wcstombs(buf, w_buf, buflen);
st->print("Time: %s %s", timestring, buf);
} else {
st->print("Time: %s", timestring);
}
} else {
st->print("Time: %s", timestring);
}
Expand Down