From c6f1f2aa882771ffb3ecf12e11dbce2d0fd57652 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 23 Feb 2023 14:52:00 -0800 Subject: [PATCH 1/4] Fork the path handling code to use wide strings on Windows. --- app/src/filesystem.h | 27 +++++++++++++++++-- app/src/filesystem_apple.mm | 2 +- app/src/filesystem_desktop_linux.cc | 4 +-- app/src/filesystem_desktop_windows.cc | 6 ++--- .../heartbeat/heartbeat_storage_desktop.cc | 19 +++++++------ app/src/heartbeat/heartbeat_storage_desktop.h | 5 ++-- 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/app/src/filesystem.h b/app/src/filesystem.h index bf55f3fa8b..83dd4c7a8c 100644 --- a/app/src/filesystem.h +++ b/app/src/filesystem.h @@ -17,7 +17,13 @@ #ifndef FIREBASE_APP_SRC_FILESYSTEM_H_ #define FIREBASE_APP_SRC_FILESYSTEM_H_ +#include "app/src/include/firebase/internal/platform.h" + +#if FIREBASE_PLATFORM_WINDOWS +#include +#else #include +#endif // FIREBASE_PLATFORM_WINDOWS namespace firebase { @@ -51,8 +57,25 @@ namespace firebase { // // TODO(b/171738655): use a separate function instead of the `should_create` // flag. Use `StatusOr` for returning errors. -std::string AppDataDir(const char* app_name, bool should_create = true, - std::string* out_error = nullptr); + +#if FIREBASE_PLATFORM_WINDOWS +typedef std::wstring PathString; +#define PathStringLiteral(x) PathString(L ## x) +#define kPathStringEmpty PathString(L"") +#define kPathStringSep PathString(L"\\") +#define PathStringChar wchar +#define PathStringLiteralPrefix L +#else +typedef std::string PathString; +#define PathStringLiteral(x) PathString(x) +#define kPathStringEmpty PathString("") +#define kPathStringSep PathString("/") +#define PathStringChar char +#define PathStringLiteralPrefix +#endif // FIREBASE_PLATFORM_WINDOWS + +PathString AppDataDir(const char* app_name, bool should_create = true, + std::string* out_error = nullptr); } // namespace firebase diff --git a/app/src/filesystem_apple.mm b/app/src/filesystem_apple.mm index 5e1727aff6..72e83e3492 100644 --- a/app/src/filesystem_apple.mm +++ b/app/src/filesystem_apple.mm @@ -44,7 +44,7 @@ bool Mkdir(const std::string& path, std::string* out_error) { } // namespace -std::string AppDataDir(const char* app_name, bool should_create, std::string* out_error) { +PathString AppDataDir(const char* app_name, bool should_create, std::string* out_error) { if (!app_name || std::strlen(app_name) == 0) { if (out_error) { *out_error = "AppDataDir failed: no app_name provided"; diff --git a/app/src/filesystem_desktop_linux.cc b/app/src/filesystem_desktop_linux.cc index b6f268c55c..3d37bc44d6 100644 --- a/app/src/filesystem_desktop_linux.cc +++ b/app/src/filesystem_desktop_linux.cc @@ -80,8 +80,8 @@ bool PathExists(const std::string& path) { } // namespace -std::string AppDataDir(const char* app_name, bool should_create, - std::string* out_error) { +PathString AppDataDir(const char* app_name, bool should_create, + std::string* out_error) { if (!app_name || strlen(app_name) == 0) { if (out_error) { *out_error = "AppDataDir failed: no app_name provided"; diff --git a/app/src/filesystem_desktop_windows.cc b/app/src/filesystem_desktop_windows.cc index ceca19d6b7..89de22f3db 100644 --- a/app/src/filesystem_desktop_windows.cc +++ b/app/src/filesystem_desktop_windows.cc @@ -164,7 +164,7 @@ bool Mkdir(const std::wstring& path, std::string* out_error) { } // namespace -std::string AppDataDir(const char* app_name, bool should_create, +PathString AppDataDir(const char* app_name, bool should_create, std::string* out_error) { if (!app_name || std::strlen(app_name) == 0) { if (out_error) { @@ -210,14 +210,14 @@ std::string AppDataDir(const char* app_name, bool should_create, if (!created) return ""; } - return NativeToUtf8(current_path, out_error); + return PathString(current_path); } else { auto app_name_utf16 = Utf8ToNative(app_name, out_error); if (app_name_utf16.empty()) { return ""; } - return NativeToUtf8(base_dir + L"/" + app_name_utf16, out_error); + return PathString(base_dir + L"/" + app_name_utf16); } } diff --git a/app/src/heartbeat/heartbeat_storage_desktop.cc b/app/src/heartbeat/heartbeat_storage_desktop.cc index a9c54fb728..60175d4442 100644 --- a/app/src/heartbeat/heartbeat_storage_desktop.cc +++ b/app/src/heartbeat/heartbeat_storage_desktop.cc @@ -39,25 +39,28 @@ using com::google::firebase::cpp::heartbeat::VerifyLoggedHeartbeatsBuffer; namespace { const char kHeartbeatDir[] = "firebase-heartbeat"; +#if FIREBASE_PLATFORM_WINDOWS +const wchar_t kHeartbeatFilenamePrefix[] = L"heartbeats-"; +#else const char kHeartbeatFilenamePrefix[] = "heartbeats-"; +#endif -std::string CreateFilename(const std::string& app_id, const Logger& logger) { +PathString CreateFilename(const std::string& app_id, const Logger& logger) { std::string error; - std::string app_dir = + PathString app_dir = AppDataDir(kHeartbeatDir, /*should_create=*/true, &error); if (!error.empty()) { logger.LogError(error.c_str()); - return ""; + return kPathStringEmpty; } if (app_dir.empty()) { - return ""; + return kPathStringEmpty; } // Remove any symbols from app_id that might not be allowed in filenames. auto app_id_without_symbols = - std::regex_replace(app_id, std::regex("[/\\\\?%*:|\"<>.,;=]"), ""); - // Note: fstream will convert / to \ if needed on windows. - return app_dir + "/" + kHeartbeatFilenamePrefix + app_id_without_symbols; + std::regex_replace(app_id, std::regex(PathStringLiteral("[/\\\\?%*:|\"<>.,;=]")), kPathStringEmpty); + return app_dir + kPathStringSep + kHeartbeatFilenamePrefix + app_id_without_symbols; } } // namespace @@ -127,7 +130,7 @@ bool HeartbeatStorageDesktop::Write(const LoggedHeartbeats& heartbeats) const { return !file.fail(); } -const char* HeartbeatStorageDesktop::GetFilename() const { +const PathStringChar* HeartbeatStorageDesktop::GetFilename() const { return filename_.c_str(); } diff --git a/app/src/heartbeat/heartbeat_storage_desktop.h b/app/src/heartbeat/heartbeat_storage_desktop.h index 8796a4455a..1d8cfee50d 100644 --- a/app/src/heartbeat/heartbeat_storage_desktop.h +++ b/app/src/heartbeat/heartbeat_storage_desktop.h @@ -24,6 +24,7 @@ #include "app/logged_heartbeats_generated.h" #include "app/src/logger.h" +#include "app/src/filesystem.h" namespace firebase { namespace heartbeat { @@ -52,7 +53,7 @@ class HeartbeatStorageDesktop { // write operation fails. bool Write(const LoggedHeartbeats& heartbeats) const; - const char* GetFilename() const; + const PathStringChar* GetFilename() const; private: LoggedHeartbeats LoggedHeartbeatsFromFlatbuffer( @@ -61,7 +62,7 @@ class HeartbeatStorageDesktop { const LoggedHeartbeats& heartbeats_struct) const; // local variables for state - std::string filename_; + PathString filename_; const Logger& logger_; }; From cd33ecbbc157962cb320fc4ddac1e205aa109e87 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 23 Feb 2023 15:58:44 -0800 Subject: [PATCH 2/4] Fix header. --- app/src/filesystem.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/src/filesystem.h b/app/src/filesystem.h index 83dd4c7a8c..ce0ea27fcd 100644 --- a/app/src/filesystem.h +++ b/app/src/filesystem.h @@ -17,13 +17,9 @@ #ifndef FIREBASE_APP_SRC_FILESYSTEM_H_ #define FIREBASE_APP_SRC_FILESYSTEM_H_ -#include "app/src/include/firebase/internal/platform.h" - -#if FIREBASE_PLATFORM_WINDOWS -#include -#else #include -#endif // FIREBASE_PLATFORM_WINDOWS + +#include "app/src/include/firebase/internal/platform.h" namespace firebase { From a51cac19233aa62cc277dc7068fb908925d3fe71 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 23 Feb 2023 16:04:02 -0800 Subject: [PATCH 3/4] Format code. --- app/src/filesystem.h | 6 +++--- app/src/filesystem_desktop_linux.cc | 2 +- app/src/filesystem_desktop_windows.cc | 2 +- app/src/heartbeat/heartbeat_storage_desktop.cc | 8 +++++--- app/src/heartbeat/heartbeat_storage_desktop.h | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/src/filesystem.h b/app/src/filesystem.h index ce0ea27fcd..883dee78c3 100644 --- a/app/src/filesystem.h +++ b/app/src/filesystem.h @@ -56,7 +56,7 @@ namespace firebase { #if FIREBASE_PLATFORM_WINDOWS typedef std::wstring PathString; -#define PathStringLiteral(x) PathString(L ## x) +#define PathStringLiteral(x) PathString(L##x) #define kPathStringEmpty PathString(L"") #define kPathStringSep PathString(L"\\") #define PathStringChar wchar @@ -69,9 +69,9 @@ typedef std::string PathString; #define PathStringChar char #define PathStringLiteralPrefix #endif // FIREBASE_PLATFORM_WINDOWS - + PathString AppDataDir(const char* app_name, bool should_create = true, - std::string* out_error = nullptr); + std::string* out_error = nullptr); } // namespace firebase diff --git a/app/src/filesystem_desktop_linux.cc b/app/src/filesystem_desktop_linux.cc index 3d37bc44d6..67c3b67f91 100644 --- a/app/src/filesystem_desktop_linux.cc +++ b/app/src/filesystem_desktop_linux.cc @@ -81,7 +81,7 @@ bool PathExists(const std::string& path) { } // namespace PathString AppDataDir(const char* app_name, bool should_create, - std::string* out_error) { + std::string* out_error) { if (!app_name || strlen(app_name) == 0) { if (out_error) { *out_error = "AppDataDir failed: no app_name provided"; diff --git a/app/src/filesystem_desktop_windows.cc b/app/src/filesystem_desktop_windows.cc index 89de22f3db..a56dd48e1b 100644 --- a/app/src/filesystem_desktop_windows.cc +++ b/app/src/filesystem_desktop_windows.cc @@ -165,7 +165,7 @@ bool Mkdir(const std::wstring& path, std::string* out_error) { } // namespace PathString AppDataDir(const char* app_name, bool should_create, - std::string* out_error) { + std::string* out_error) { if (!app_name || std::strlen(app_name) == 0) { if (out_error) { *out_error = "AppDataDir failed: no app_name provided"; diff --git a/app/src/heartbeat/heartbeat_storage_desktop.cc b/app/src/heartbeat/heartbeat_storage_desktop.cc index 60175d4442..f8850228ae 100644 --- a/app/src/heartbeat/heartbeat_storage_desktop.cc +++ b/app/src/heartbeat/heartbeat_storage_desktop.cc @@ -58,9 +58,11 @@ PathString CreateFilename(const std::string& app_id, const Logger& logger) { } // Remove any symbols from app_id that might not be allowed in filenames. - auto app_id_without_symbols = - std::regex_replace(app_id, std::regex(PathStringLiteral("[/\\\\?%*:|\"<>.,;=]")), kPathStringEmpty); - return app_dir + kPathStringSep + kHeartbeatFilenamePrefix + app_id_without_symbols; + auto app_id_without_symbols = std::regex_replace( + app_id, std::regex(PathStringLiteral("[/\\\\?%*:|\"<>.,;=]")), + kPathStringEmpty); + return app_dir + kPathStringSep + kHeartbeatFilenamePrefix + + app_id_without_symbols; } } // namespace diff --git a/app/src/heartbeat/heartbeat_storage_desktop.h b/app/src/heartbeat/heartbeat_storage_desktop.h index 1d8cfee50d..9f7d7c2760 100644 --- a/app/src/heartbeat/heartbeat_storage_desktop.h +++ b/app/src/heartbeat/heartbeat_storage_desktop.h @@ -23,8 +23,8 @@ #include #include "app/logged_heartbeats_generated.h" -#include "app/src/logger.h" #include "app/src/filesystem.h" +#include "app/src/logger.h" namespace firebase { namespace heartbeat { From d5545f881fcacb2fe54a7d41c44ed599d6033359 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 23 Feb 2023 16:21:32 -0800 Subject: [PATCH 4/4] Fix wchar_t type. --- app/src/filesystem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/filesystem.h b/app/src/filesystem.h index 883dee78c3..0e22929509 100644 --- a/app/src/filesystem.h +++ b/app/src/filesystem.h @@ -59,7 +59,7 @@ typedef std::wstring PathString; #define PathStringLiteral(x) PathString(L##x) #define kPathStringEmpty PathString(L"") #define kPathStringSep PathString(L"\\") -#define PathStringChar wchar +#define PathStringChar wchar_t #define PathStringLiteralPrefix L #else typedef std::string PathString;