This PR has been migrated from old Barrier Github repository debauchee/barrier#979

PR created on: 2020-12-09 by @albertony
PR last updated on: 2020-12-30

Fixes debauchee/barrier#976
Fixes debauchee/barrier#974
Fixes debauchee/barrier#444

Probably conflicts with debauchee/barrier#972, but if you accept both PRs then just "ping" me and I can quickly fix it (either if I can merge the PR branches together, or you can merge one of them to master first and I rebase the other one, or something else..?)

Commented on: 2020-12-09 by @p12tic

I think it makes sense to have these as simple functions.

Commented on: 2020-12-09 by @albertony

Pushed an update, but not sure if it was something like this you had in mind, or something else? (and if so, I'm happy to make another attempt)

Commented on: 2020-12-10 by @albertony

Pushed another update, because I managed to track down another one of those codepage issues. Refactored the KnowFolderPaths into a header-only, and added another one, StringConverters.h, for windows string conversions (narrow ansi, narrow utf8, and wide). But again, just let me know if you think it would be better doing it differently!

Commented on: 2020-12-10 by @p12tic

static is incorrect for functions that are in the headers and are supposed to be reused in many cpp files. In such case there's a separate copy of the function in each translation unit and we rely on the linker to deduplicate.

Better to use inline.

Even better would be to add a .cpp file for the implementations.

The same applies to a header below.

Commented on: 2020-12-10 by @albertony

Better than ever? :)

Commented on: 2020-12-11 by @albertony

Tracked down a couple of more issues.. There were multiple similar problems, in client, server and deamon, so it took several attempts to track them down and fix them. Not entirely sure all are properly handled yet, I hope so, and think so, but let's wait a little bit and see if @cheese, reporter of debauchee/barrier#974, will agree...

In general I guess the main solution is to be explicit about use of UTF-8 for strings sent over IPC, and to be sure to use current locale encoding (normally "ANSI" on Windows) for std::string's. E.g. QString::toStdString uses UTF-8, which is somewhat annoying (although UTF-8 Everywhere is a thing, its not a short term solution here).

Edit: Wait a minute.. For the QString conversions I can probably use Qt's to/fromLocal8Bit. I considered those before, but wasn't sure they were matching the MultiByte/ANSI/codepage/whatever-you-call-it kind of strings on Windows, but then found this:

The local 8-bit codec can convert from unicode to the character set specified in the locale and vice versa.
This codec is called the "System" codec. This codec can be obtained using QTextCodec::codecForLocale()
or QTextCodec::codecForName("System").

On Windows, the "System" QTextCodec uses MultiByteToWideChar and WideCharToMultiByte (with
CP_ACP - ANSI code page) to convert to and from unicode.

On Unix, coversion is done using iconv. However, Qt can be compiled without iconv support, in which
case Qt inspects the LANG, LC_CTYPE, LC_ALL environment variables to determine the codec. The codec
detected has to be a part of the Qt built-in codec list (Otherwise, it defaults to latin-1).

Will rewrite (again) and post back...

Commented on: 2020-12-12 by @albertony

Done.

• Consistent use of std::string with standard locale-specific encoding.
• IPC messages always encode strings with UTF-8, which are converted back and forth at each end.
• In GUI layer, using Qt's QString::fromLocal8Bit/QString::toLocal8Bit to convert between (unicode) QString and (locale-specific) std::string, and QString::fromUtf8/QString:toUtf8 for reading/writing UTF-8 encoded strings for IPC communication. So encoding is quite explicit. Not using toStdString/fromStdString, since it returns std::string that are UTF-8 encoded and leads to confusion and problems.
• In kernel: Locale encoded std::string, using the existing Unicode::UTF8ToText/Unicode::textToUTF8 for UTF-8 specific IPC strings (except in this one place, in DataDirectories, since it is in common lib and do not have access to Unicode class in base lib).

PS: Would still be nice to see #974 being tested OK, since there were so many iterations with different issues here..

Commented on: 2020-12-30 by @p12tic

Thanks!

Closed on: 2020-12-30