Skip to content
Closed
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions test/hotspot/gtest/compiler/test_directivesParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@

class DirectivesParserTest : public ::testing::Test{
protected:
const char* const _locale;
char* const _locale;
ResourceMark rm;
stringStream stream;
// These tests require the "C" locale to correctly parse decimal values
DirectivesParserTest() : _locale(setlocale(LC_NUMERIC, nullptr)) {
DirectivesParserTest() : _locale(os::strdup(setlocale(LC_NUMERIC, nullptr), mtTest)) {
setlocale(LC_NUMERIC, "C");
Comment on lines +38 to 39
Copy link
Member

Choose a reason for hiding this comment

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

Would it fix the issue if we did this instead?

Suggested change
DirectivesParserTest() : _locale(os::strdup(setlocale(LC_NUMERIC, nullptr), mtTest)) {
setlocale(LC_NUMERIC, "C");
DirectivesParserTest() : _locale(setlocale(LC_NUMERIC, "C")) {

seems to me that the string returned by setlocale is only valid until the next setlocale call, and currently we call setlocale twice in the constructor, and save the result of the first call.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The first setlocate call returns the pointer to the last locale, which becomes invalid. Changing the input string on the first setlocale call won't change that.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. I was misled by the setlocale docs:

The string returned is such that a subsequent call with that string and its associated category will restore that part of the process's locale.

apparently it doesn't restore them to the previous value, as I incorrectly assumed.

}
~DirectivesParserTest() {
setlocale(LC_NUMERIC, _locale);
os::free(_locale);
}

void test_negative(const char* text) {
Expand Down