Skip to content

console: fix getwchar failing when LC_ALL undefined #3688

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

console: fix getwchar failing when LC_ALL undefined #3688

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9eab8b6
fix getwchar failing when LC_ALL undefined
staviq Oct 19, 2023
5616b43
fmt
staviq Oct 19, 2023
1e328f4
fix non-utf locale
staviq Oct 20, 2023
1ad5224
fix non-utf locale 2
staviq Oct 20, 2023
a72c053
Update common/console.cpp
staviq Oct 20, 2023
8396208
remove redundant C locale check
staviq Oct 20, 2023
226ed5f
fix empty str vs nullptr for setlocale and getenv
staviq Oct 20, 2023
6d126d0
fix win
staviq Oct 20, 2023
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
27 changes: 27 additions & 0 deletions common/console.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <stdlib.h>
#include <signal.h>
#include <termios.h>
#include <string.h>
#endif

#define ANSI_COLOR_RED "\x1b[31m"
Expand Down Expand Up @@ -52,6 +53,21 @@ namespace console {
static termios initial_state;
#endif

#if !defined(_WIN32)
static bool locale_isvalid( const char * l )
{
return !(l == nullptr || l[0] == '\0' || (strcasestr(l, "utf-8") == nullptr && strcasestr(l, "utf8") == nullptr));
}

static bool locale_setverify( int c, const char * l )
{
setlocale(c, l);
auto locale = setlocale(c, nullptr);

return !strcmp( l, locale );
}
#endif
Comment on lines +62 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out setlocale(X,"") is not the same as setlocale(X,nullptr) as the former only returns nullptr if locale changes, whereas the latter returns but doesn't change.

You are right about passing in NULL for the second argument, however, you are wrong about how it works when you pass in a string. setlocale will only return nullptr if it fails to set the locale. It will also always return the current locale if it succeeds, and it will succeed even if you pass it in the current locale and nothing changes. So there is no need for a separate call to check the results. You can simply look at the return results.

locale_setverify is unnecessary at best and wrong at worst. It won't work correctly when you pass in l of empty string ("") for example - which getenv("LANG") could absolutely be.


//
// Init and cleanup
//
Expand Down Expand Up @@ -112,6 +128,17 @@ namespace console {
}

setlocale(LC_ALL, "");
auto locale = setlocale(LC_ALL, nullptr);

if (!locale_isvalid(locale)) {
auto lang = getenv("LANG");
if (!locale_isvalid(lang) || !locale_setverify(LC_ALL, lang)) {
locale_setverify(LC_ALL, "C.UTF-8") || locale_setverify(LC_ALL, "C.utf8") || setlocale(LC_ALL, "");
}
}

auto usedlocale = setlocale(LC_ALL, nullptr);
fprintf(stderr,"Console: using '%s' locale.\n", usedlocale);
#endif
}

Expand Down