glog icon indicating copy to clipboard operation
glog copied to clipboard

EnableLogCleaner doesn't work

Open ml232528 opened this issue 3 years ago • 6 comments

glog v0.5.0 EnableLogCleaner doesn't work because opendir only supports English paths. Need to call setlocale (lc_all, ""); Can be used correctly。

ml232528 avatar Jan 24 '22 06:01 ml232528

Could you please provide a minimal example that shows the problem?

sergiud avatar Jan 24 '22 09:01 sergiud

https://github.com/google/glog/blob/b0174b3dda95be3858ec54570c1375f2f00e984e/src/logging.cc#L62-L66 https://github.com/google/glog/blob/b0174b3dda95be3858ec54570c1375f2f00e984e/src/logging.cc#L1359

On Unix systems, opendir function should just pass raw bytes to kernel, without being affected by locale. For Windows, glog defines custom version of opendir function: https://github.com/google/glog/blob/b0174b3dda95be3858ec54570c1375f2f00e984e/src/windows/dirent.h#L672-L726 According to documentation, Windows version of mbstowcs_s function (called by dirent_mbstowcs_s function) supposedly uses current locale.

@ml232528: Do you use Windows?

setlocale (LC_ALL, ""), or rather setlocale (LC_CTYPE, "") if it is sufficient and working on Windows, should be probably in Windows-specific block of code somewhere:

#ifdef GLOG_OS_WINDOWS
  setlocale(LC_CTYPE, "");
#endif

Arfrever avatar Jan 24 '22 21:01 Arfrever

@ml232528 It is not clear what kind of resolution do you expect here. Could you please also respond to @Arfrever's questions?

sergiud avatar Feb 14 '22 11:02 sergiud

If that setlocale (LC_CTYPE, "") was to be added somewhere, probably appropriate place would be glog_internal_namespace_::InitGoogleLoggingUtilities (which is called by google::InitGoogleLogging).

I do not use Windows, so here is untested patch:

--- src/utilities.cc
+++ src/utilities.cc
@@ -57,6 +57,9 @@
 #ifdef __ANDROID__
 #include <android/log.h>
 #endif
+#ifdef GLOG_OS_WINDOWS
+# include <clocale>  // For setlocale()
+#endif
 
 #include "base/googleinit.h"
 
@@ -364,6 +367,11 @@
 void InitGoogleLoggingUtilities(const char* argv0) {
   CHECK(!IsGoogleLoggingInitialized())
       << "You called InitGoogleLogging() twice!";
+
+#ifdef GLOG_OS_WINDOWS
+  setlocale(LC_CTYPE, "");
+#endif
+
   const char* slash = strrchr(argv0, '/');
 #ifdef GLOG_OS_WINDOWS
   if (!slash)  slash = strrchr(argv0, '\\');

Arfrever avatar Feb 14 '22 18:02 Arfrever

It is recommended to replace mbstowcs with multibytetowidechar and use the flag CP_ ACP,Example:

std::wstring As_WStringFromString(std::string const &s) {
	auto len = ::MultiByteToWideChar(CP_ACP, 0, s.c_str(), s.size(), NULL, 0);
	std::unique_ptr<WCHAR[]> result(new WCHAR[len + 1]);
	::MultiByteToWideChar(CP_ACP, 0, s.c_str(), s.size(), result.get(), len + 1);
	result.get()[len] = 0;
	return std::wstring(result.get());
}


std::string As_StringFromWstring(std::wstring const &s) {
	auto len = WideCharToMultiByte(CP_ACP, 0, s.c_str(), s.size(), nullptr, 0, nullptr, nullptr);
	std::unique_ptr<char[]> result(new char[len + 1]);
	WideCharToMultiByte(CP_ACP, 0, s.c_str(), s.size(), result.get(), len + 1, nullptr, nullptr);
	result.get()[len] = 0;
	return std::string(result.get());
}

std::string As_Utf8FromWstring(std::wstring const &s) {
	auto len = WideCharToMultiByte(CP_UTF8, 0, s.c_str(), s.size(), nullptr, 0, nullptr, nullptr);
	std::unique_ptr<char[]> result(new char[len + 1]);
	WideCharToMultiByte(CP_UTF8, 0, s.c_str(), s.size(), result.get(), len + 1, nullptr, nullptr);
	result.get()[len] = 0;
	return std::string(result.get());
}

ml232528 avatar Feb 18 '22 02:02 ml232528

Maybe you could create pull-request for glog? (You would need to firstly sign Google's Contributor License Agreement if not done already.)

Arfrever avatar Feb 18 '22 11:02 Arfrever