From 1725e1010413a9967f9d8e59c41c9e335bf6111f Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Mon, 30 Jan 2017 20:42:35 -0800 Subject: [PATCH 1/8] Adding path mapper tests before refactoring the Ebr file code Remove set/get cwd, which are unused --- Frameworks/Starboard/AssetFile.cpp | 4 +- Frameworks/Starboard/EbrFile.cpp | 20 +-- Frameworks/Starboard/PathMapper.h | 14 -- Frameworks/include/Platform/EbrPlatform.h | 3 - build/Starboard/dll/Starboard.def | 2 - .../Starboard/Starboard.UnitTests.vcxproj | 9 +- tests/unittests/Starboard/PathMapperTests.mm | 137 ++++++++++++++++++ 7 files changed, 147 insertions(+), 42 deletions(-) create mode 100644 tests/unittests/Starboard/PathMapperTests.mm diff --git a/Frameworks/Starboard/AssetFile.cpp b/Frameworks/Starboard/AssetFile.cpp index 13c7659fa7..d5bb5abd61 100644 --- a/Frameworks/Starboard/AssetFile.cpp +++ b/Frameworks/Starboard/AssetFile.cpp @@ -32,8 +32,6 @@ static const wchar_t* TAG = L"AssetFile"; #define strtok_r strtok_s -char CPathMapper::currentDir[4096]; - void appendPath(char* curpath, const char* path) { char copy[4096]; @@ -199,7 +197,7 @@ CPathMapper::CPathMapper(const char* path) { strcpy_s(relativePath, ""); if (path[0] != '/') { - appendPath(relativePath, currentDir); + appendPath(relativePath, ""); } appendPath(relativePath, path); fixedValid = fixPath(fixedPath, relativePath); diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index 93a483c532..6f337d86a3 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -37,7 +37,7 @@ static const wchar_t* TAG = L"EbrFile"; std::mutex EbrFile::s_fileMapLock{}; std::map> EbrFile::s_fileMap{}; -int EbrFile::s_maxFileId{0}; +int EbrFile::s_maxFileId{ 0 }; std::shared_ptr EbrFile::GetFile(int fid) { std::lock_guard guard(s_fileMapLock); @@ -80,17 +80,16 @@ int EbrOpen(const char* file, int mode, int share) { } int EbrOpenWithPermission(const char* file, int mode, int share, int pmode) { - std::shared_ptr fileToAdd; // Special random number device. Just a stub. fileToAdd = EbrDevRandomFile::CreateInstance(file, mode, share, pmode); - + if (!fileToAdd) { // Special file type for cached storage files. fileToAdd = EbrStorageFile::CreateInstance(file, mode, share, pmode); - } - + } + if (!fileToAdd) { // No more special types. Assume its a real file. fileToAdd = EbrIOFile::CreateInstance(file, mode, share, pmode); @@ -226,17 +225,6 @@ bool EbrMkdir(const char* path) { return _mkdir(CPathMapper(path)) == 0; } -char* EbrGetcwd(char* buf, size_t len) { - strncpy_s(buf, len, CPathMapper::currentDir, len); - return buf; -} - -int EbrChdir(const char* path) { - CPathMapper::setCWD(path); - - return 0; -} - int EbrChmod(const char* path, int mode) { return _chmod(CPathMapper(path), mode); } diff --git a/Frameworks/Starboard/PathMapper.h b/Frameworks/Starboard/PathMapper.h index 96b3745298..4135014b60 100644 --- a/Frameworks/Starboard/PathMapper.h +++ b/Frameworks/Starboard/PathMapper.h @@ -20,20 +20,6 @@ class CPathMapper { char fixedPath[4096]; bool fixedValid, mappedValid; - static char currentDir[4096]; - - static void setCWD(const char* directory) { - if (directory[0] != '/') { - strcat_s(currentDir, directory); - } else { - strcpy_s(currentDir, directory); - } - } - - static void getCWD(char* directory) { - strcpy_s(directory, 4095, currentDir); - } - char* FixedPath(); char* MappedPath(); diff --git a/Frameworks/include/Platform/EbrPlatform.h b/Frameworks/include/Platform/EbrPlatform.h index b81ce4b44c..0920a781e4 100644 --- a/Frameworks/include/Platform/EbrPlatform.h +++ b/Frameworks/include/Platform/EbrPlatform.h @@ -50,8 +50,6 @@ SB_EXPORT int EbrTruncate64(int fd, __int64 size); SB_EXPORT bool EbrRename(const char* path1, const char* path2); SB_EXPORT bool EbrUnlink(const char* path); SB_EXPORT bool EbrMkdir(const char* path); -SB_EXPORT char* EbrGetcwd(char* buf, size_t len); -SB_EXPORT int EbrChdir(const char* path); SB_EXPORT int EbrChmod(const char* path, int mode); @@ -107,4 +105,3 @@ typedef struct { SB_EXPORT int EbrEventTimedMultipleWait(EbrEvent* events, int numEvents, double timeout, SocketWait* sockets); SB_EXPORT void EbrEventDestroy(EbrEvent event); - diff --git a/build/Starboard/dll/Starboard.def b/build/Starboard/dll/Starboard.def index 53148056e7..521c731e67 100644 --- a/build/Starboard/dll/Starboard.def +++ b/build/Starboard/dll/Starboard.def @@ -154,8 +154,6 @@ LIBRARY Starboard EbrRename EbrUnlink EbrMkdir - EbrGetcwd - EbrChdir EbrChmod EbrRemove EbrRemoveEmptyDir diff --git a/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj b/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj index cab68b0da0..bc08440a9f 100644 --- a/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj +++ b/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj @@ -140,7 +140,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp -Wdeprecated-declarations SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;DEBUG=1;%(PreprocessorDefinitions) @@ -162,7 +162,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp -Wdeprecated-declarations SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;DEBUG=1;%(PreprocessorDefinitions) @@ -188,7 +188,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;%(PreprocessorDefinitions) MultiThreadedDLL @@ -214,7 +214,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;%(PreprocessorDefinitions) MultiThreadedDLL @@ -237,6 +237,7 @@ + CompileAsObjC diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm new file mode 100644 index 0000000000..ea0a64e7ea --- /dev/null +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -0,0 +1,137 @@ +//****************************************************************************** +// +// Copyright (c) 2016 Microsoft Corporation. All rights reserved. +// +// This code is licensed under the MIT License (MIT). +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//****************************************************************************** + +#include +#include "pathmapper.h" + +bool fixPath(char* outPath, const char* relativePath); +bool convertPath(char* filePath, const char* relativePath); + +TEST(PathMapper, fixPathNegativeTests) { + char outPath[4096]; + EXPECT_FALSE(fixPath(outPath, "must start with /")); + EXPECT_EQ_MSG(outPath[0], 0, outPath); + + EXPECT_FALSE(fixPath(outPath, "/..")); + EXPECT_EQ_MSG(outPath[0], 0, outPath); + + EXPECT_FALSE(fixPath(outPath, "/./..")); + EXPECT_EQ_MSG(outPath[0], 0, outPath); + + EXPECT_FALSE(fixPath(outPath, "/./src/../../..")); + // this test was failing before, it is a bug + EXPECT_EQ_MSG(outPath[0], 0, outPath); + + EXPECT_FALSE(fixPath(outPath, "/src/../..")); + // this test was failing before, it is a bug + EXPECT_EQ_MSG(outPath[0], 0, outPath); +} + +TEST(PathMapper, fixPathPositiveTests) { + char outPath[4096]; + EXPECT_TRUE(fixPath(outPath, "/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "//src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "///src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "/./src")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "/d:/src")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/d:/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "/./src/../src/")); + // fails, returns //src + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "/./src/winobjc/..")); + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + + EXPECT_TRUE(fixPath(outPath, "/./././src/winobjc/../winobjc/.././../src/winobjc/..")); + // fails, returns //src + EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); +} + +extern "C" void EbrSetWritableFolder(const char*); + +TEST(PathMapper, mapPathPositiveTests) { + EbrSetWritableFolder("d:\\temp"); + + char outPath[4096]; + EXPECT_TRUE(convertPath(outPath, "/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, ".\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "")); + EXPECT_EQ_MSG(0, strcmp(outPath, "."), outPath); + + EXPECT_TRUE(convertPath(outPath, "src")); + EXPECT_EQ_MSG(0, strcmp(outPath, ".\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "c:/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "c:\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/c:/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "c:\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/X:/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "X:\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/Documents/src/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Documents\\src"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/Documents/./")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Documents"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/Cache/test/.")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\cache\\test"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/library")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Library"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/AppSupport/././")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\AppSupport"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/tmp")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\tmp"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/shared")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\shared"), outPath); + + EXPECT_TRUE(convertPath(outPath, "/AppSupport/.\\?/")); + EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\AppSupport\\+"), outPath); +} + +TEST(PathMapper, mapPathNegativeTests) { + char outPath[4096]; + + // first slash must be / slash, is this just an artifact? + EXPECT_TRUE(convertPath(outPath, "\\AppSupport\\.\\?/")); + // fails previously + EXPECT_EQ_MSG(0, strcmp(outPath, ".\\AppSupport\\+"), outPath); +} + +TEST(PathMapper, pathMapper) { + CPathMapper mapper1("c:/users/winobjc-bot"); + EXPECT_EQ_MSG(0, strcmp(mapper1, "c:\\users\\winobjc-bot"), (const char*)mapper1); + EXPECT_EQ_MSG(0, strcmp(mapper1.fixedPath, "/c:/users/winobjc-bot"), (const char*)mapper1.fixedPath); + + CPathMapper mapper2("~/winobjc-bot"); + EXPECT_EQ_MSG(0, strcmp(mapper2, ".\\home\\winobjc-bot"), (const char*)mapper2); +} From e238a98f6d023fdc236451f2f738c2ae548ef6a2 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Tue, 31 Jan 2017 13:05:24 -0800 Subject: [PATCH 2/8] * Rewrite path mapper to simplify code and support UTF16 path names. * Rewrite Ebr file apis to use new path mapper. * Remove dead code. * fixed unit tests to build after refactor. Fixes #1875 --- Frameworks/Starboard/AssetFile.cpp | 282 +++---------------- Frameworks/Starboard/EbrFile.cpp | 62 +--- Frameworks/Starboard/EbrIOFile.cpp | 3 +- Frameworks/Starboard/PathMapper.cpp | 136 +++++++++ Frameworks/Starboard/PathMapper.h | 42 ++- Frameworks/include/Platform/EbrPlatform.h | 2 - build/Starboard/dll/Starboard.def | 1 - build/Starboard/lib/StarboardLib.vcxproj | 3 +- tests/unittests/Starboard/PathMapperTests.mm | 148 ++++------ 9 files changed, 271 insertions(+), 408 deletions(-) create mode 100644 Frameworks/Starboard/PathMapper.cpp diff --git a/Frameworks/Starboard/AssetFile.cpp b/Frameworks/Starboard/AssetFile.cpp index d5bb5abd61..ed470fd077 100644 --- a/Frameworks/Starboard/AssetFile.cpp +++ b/Frameworks/Starboard/AssetFile.cpp @@ -14,226 +14,39 @@ // //****************************************************************************** -#include -#include -#include -#include +#include "Starboard.h" +#include "ErrorHandling.h" +#include "StringHelpers.h" #include -#include -#include -#include "Starboard.h" -#include "Platform/EbrPlatform.h" #include "AssetFile.h" #include "PathMapper.h" -#include "LoggingNative.h" static const wchar_t* TAG = L"AssetFile"; -#define strtok_r strtok_s - -void appendPath(char* curpath, const char* path) { - char copy[4096]; - - strcpy_s(copy, path); - - char* save; - char* curToken = strtok_r(copy, "/\\", &save); - char* curpathEnd = curpath + strlen(curpath); - while (curToken) { - if (strlen(curToken) == 0) { - curToken = strtok_r(NULL, "/\\", &save); - } - int tokenLen = strlen(curToken); - if (strcmp(curToken, "~") == 0) { - strcpy_s(curpath, 2048, "/home"); - curpathEnd = curpath + strlen(curpath); - } else { - strcat_s(curpathEnd, 2048, "/"); - curpathEnd++; - strcat_s(curpathEnd, 2048, curToken); - curpathEnd += tokenLen; - } - curToken = strtok_r(NULL, "/\\", &save); - } - if (strcmp(curpath, "") == 0) - strcpy_s(curpath, 2048, "/"); -} - -static void EscapePath(char* dest, const char* src) { - while (*dest) - dest++; - - while (*src) { - switch (*src) { - case '?': - *dest = '+'; - break; - - default: - *dest = *src; - } - - dest++; - src++; - } - - *dest = 0; -} - -bool convertPath(char* filePath, const char* relativePath) { - if (!EbrGetRootMapping(NULL, filePath, 4096)) - return false; - int curComponent = 0; - - char copy[4096]; - - strcpy_s(copy, relativePath); - - char* save; - char* curToken = strtok_r(copy, "/", &save); - char* filePathEnd = filePath + strlen(filePath); - while (curToken) { - if (strlen(curToken) == 0 || strcmp(curToken, ".") == 0) { - curToken = strtok_r(NULL, "/", &save); - continue; - } - - if (curComponent == 0) { - if (!EbrGetRootMapping(curToken, filePath, 4096)) - return false; - filePathEnd = filePath + strlen(filePath); - } else { - strcat_s(filePathEnd, 2048, "\\"); - filePathEnd++; - EscapePath(filePathEnd, curToken); - filePathEnd += strlen(curToken); - } - curComponent++; - - curToken = strtok_r(NULL, "/\\", &save); - } - - if (strcmp(filePath, "") == 0) - return false; - - return true; -} - -bool fixPath(char* outPath, const char* relativePath) { - strcpy_s(outPath, 2048, ""); - int curComponent = 0; - - if (relativePath[0] != '/') - return false; - - char copy[4096]; - - strcpy_s(copy, relativePath); - - char* save; - char* curToken = strtok_r(copy, "/", &save); - char* outPathEnd = outPath + strlen(outPath); - while (curToken) { - if (strlen(curToken) == 0 || strcmp(curToken, ".") == 0) { - curToken = strtok_r(NULL, "/", &save); - continue; - } - if (strcmp(curToken, "..") == 0) { - if (curComponent == 0) { - return false; - } - // Move back one directory - char* curPos = outPath + strlen(outPath); - while (curPos >= outPath) { - if (*curPos == '/') { - *curPos = 0; - break; - } - curPos--; - } - // Impossible! - if (curPos < outPath) { - return false; - } - if (curPos == outPath) { - strcpy_s(outPath, 2048, "/"); - } - curComponent--; - curToken = strtok_r(NULL, "/", &save); - outPathEnd = outPath + strlen(outPath); - continue; - } - - strcat_s(outPathEnd, 2048, "/"); - outPathEnd++; - strcat_s(outPathEnd, 2048, curToken); - outPathEnd += strlen(curToken); - curComponent++; - - curToken = strtok_r(NULL, "/", &save); - } - if (strcmp(outPath, "") == 0) { - strcpy_s(outPath, 2048, "/"); - } - - return true; -} - -char* CPathMapper::FixedPath() { - if (fixedValid) - return fixedPath; - return NULL; -} - -char* CPathMapper::MappedPath() { - if (fixedValid && mappedValid) - return mappedPath; - return NULL; -} - -CPathMapper::CPathMapper(const char* path) { - char relativePath[4096]; - strcpy_s(relativePath, ""); - - if (path[0] != '/') { - appendPath(relativePath, ""); - } - appendPath(relativePath, path); - fixedValid = fixPath(fixedPath, relativePath); - mappedValid = convertPath(mappedPath, fixedPath); -} - -void ScanAssets() { -} - struct EbrDir { EbrDirReader* curReader; }; class EbrFSDirReader : public EbrDirReader { public: - char path[4096]; - char startPath[4096]; + std::wstring path; + std::wstring startPath; HANDLE findHandle; WIN32_FIND_DATAW data; bool isFirst; - static EbrDirReader* open(const char* path); + static EbrDirReader* open(const std::wstring& path); virtual ~EbrFSDirReader(); virtual bool readNext(EbrDir* curDir, EbrDirEnt* end); }; -EbrDirReader* EbrFSDirReader::open(const char* path) { - CPathMapper map(path); - if (!map.MappedPath()) - return NULL; - +EbrDirReader* EbrFSDirReader::open(const std::wstring& path) { EbrFSDirReader* ret = new EbrFSDirReader(); - sprintf_s(ret->path, "%s\\*", (const char*)CPathMapper(path)); - strcpy_s(ret->startPath, path); - std::wstring widePath(ret->path, ret->path + strlen(ret->path)); - ret->findHandle = FindFirstFileExW(widePath.c_str(), FindExInfoStandard, &ret->data, FindExSearchNameMatch, NULL, 0); + ret->path = path + std::wstring(L"\\*"); + ret->startPath = path; + + ret->findHandle = FindFirstFileExW(ret->path.c_str(), FindExInfoStandard, &ret->data, FindExSearchNameMatch, NULL, 0); if (!ret->findHandle || ret->findHandle == INVALID_HANDLE_VALUE) { delete ret; return NULL; @@ -257,29 +70,19 @@ bool EbrFSDirReader::readNext(EbrDir* curDir, EbrDirEnt* ent) { return false; } - // Note that we're doing wcslen here, which is number of characters. If we've got continues - // in the stream then we may truncate the buffer. - std::string conv(data.cFileName, data.cFileName + wcslen(data.cFileName)); - strcpy_s(ent->fileName, conv.c_str()); - char tmpPath[4096]; - strcpy_s(tmpPath, startPath); - strcat_s(tmpPath, "//"); - strcat_s(tmpPath, conv.c_str()); + const std::string filename = Strings::WideToNarrow(std::wstring(data.cFileName)); + strcpy_s(ent->fileName, filename.c_str()); ent->isDir = (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY; return true; } EbrDir* EbrOpenDir(const char* path) { CPathMapper map(path); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return NULL; - } - if (!map.MappedPath()) + if (!map) return NULL; - EbrDirReader* fsReader = EbrFSDirReader::open(fixedName); + EbrDirReader* fsReader = EbrFSDirReader::open(map.MappedPath()); if (!fsReader) { return NULL; } @@ -300,75 +103,64 @@ void EbrCloseDir(EbrDir* pDir) { delete pDir; } -bool EbrIsDir(const char* path) { - CPathMapper map(path); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return NULL; - } - - std::wstring unicodePath(map.MappedPath(), map.MappedPath() + strlen(map.MappedPath())); +static bool _EbrIsDir(const wchar_t* path) { WIN32_FILE_ATTRIBUTE_DATA fileAttribData; - if (GetFileAttributesExW(unicodePath.c_str(), GetFileExInfoStandard, &fileAttribData)) { + if (GetFileAttributesExW(path, GetFileExInfoStandard, &fileAttribData)) { if ((fileAttribData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY) return true; } return false; } +bool EbrIsDir(const char* path) { + CPathMapper map(path); + + if (!map) { + return NULL; + } + + return _EbrIsDir(map); +} + int EbrStat(const char* filename, struct stat* ret) { CPathMapper map(filename); - char* fixedName = map.FixedPath(); - if (!fixedName) { + if (!map) { + TraceError(TAG, L"EbrStat failure!"); return -1; } - if (EbrIsDir(filename)) { + if (_EbrIsDir(map)) { memset(ret, 0, sizeof(struct stat)); ret->st_size = 0; ret->st_mode = 0x1B6 | 0040000; return 0; } - if (!map.MappedPath()) { - TraceError(TAG, L"EbrStat failure!"); - return -1; - } - - return stat(map.MappedPath(), ret); + return _wstat32(map, reinterpret_cast(ret)); } int EbrStat64i32(const char* filename, struct _stat64i32* ret) { CPathMapper map(filename); - char* fixedName = map.FixedPath(); - if (!fixedName) { + if (!map) { + TraceError(TAG, L"EbrStat failure!"); return -1; } - if (EbrIsDir(filename)) { + if (_EbrIsDir(map)) { memset(ret, 0, sizeof(struct _stat64i32)); ret->st_size = 0; ret->st_mode = 0x1B6 | 0040000; return 0; } - if (!map.MappedPath()) { - TraceError(TAG, L"EbrStat failure!"); - return -1; - } - - return _stat64i32(map.MappedPath(), ret); + return _wstat64i32(map, ret); } int EbrAccess(const char* file, int mode) { CPathMapper map(file); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return -1; - } - if (!map.MappedPath()) + if (!map) return -1; - return _access(map.MappedPath(), mode); + return _waccess(map, mode); } diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index 6f337d86a3..ddbd81a0f0 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -149,18 +149,17 @@ int EbrFflush(int fd) { } bool EbrRemoveEmptyDir(const char* path) { - return RemoveDirectoryA(CPathMapper(path)); + return RemoveDirectoryW(CPathMapper(path)); } bool EbrRename(const char* path1, const char* path2) { - return rename(CPathMapper(path1), CPathMapper(path2)) == 0; + return _wrename(CPathMapper(path1), CPathMapper(path2)) == 0; } bool EbrUnlink(const char* path) { - return _unlink(CPathMapper(path)) == 0; + return _wunlink(CPathMapper(path)) == 0; } -#define FSROOT "." #define mkdir _mkdir char g_WritableFolder[2048] = "."; @@ -172,61 +171,12 @@ const char* EbrGetWritableFolder() { return g_WritableFolder; } -bool EbrGetRootMapping(const char* dirName, char* dirOut, uint32_t maxLen) { - if (dirName == NULL) { - strcpy_s(dirOut, maxLen, FSROOT); - return true; - } - if (_stricmp(dirName, "Documents") == 0) { - sprintf_s(dirOut, maxLen, "%s\\Documents", g_WritableFolder); - mkdir(dirOut); - - char tmpDir[4096]; - strcpy_s(tmpDir, dirOut); - strcat_s(tmpDir, "\\Library"); - mkdir(tmpDir); - return true; - } - if (_stricmp(dirName, "Cache") == 0) { - sprintf_s(dirOut, maxLen, "%s\\cache", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "Library") == 0) { - sprintf_s(dirOut, maxLen, "%s\\Library", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "AppSupport") == 0) { - sprintf_s(dirOut, maxLen, "%s\\AppSupport", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "tmp") == 0) { - sprintf_s(dirOut, maxLen, "%s\\tmp", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "shared") == 0) { - sprintf_s(dirOut, maxLen, "%s\\shared", g_WritableFolder); - mkdir(dirOut); - return true; - } - static std::regex drive("[a-zA-Z]:"); - if (std::regex_match(dirName, drive)) { - sprintf_s(dirOut, maxLen, dirName); - return true; - } - sprintf_s(dirOut, maxLen, FSROOT "\\%s", dirName); - return true; -} - bool EbrMkdir(const char* path) { - return _mkdir(CPathMapper(path)) == 0; + return _wmkdir(CPathMapper(path)) == 0; } int EbrChmod(const char* path, int mode) { - return _chmod(CPathMapper(path), mode); + return _wchmod(CPathMapper(path), mode); } #define PATH_SEPARATOR "/" @@ -267,4 +217,4 @@ bool EbrRemove(const char* path) { } } return false; -} \ No newline at end of file +} diff --git a/Frameworks/Starboard/EbrIOFile.cpp b/Frameworks/Starboard/EbrIOFile.cpp index 390eabb7b8..e8cef67004 100644 --- a/Frameworks/Starboard/EbrIOFile.cpp +++ b/Frameworks/Starboard/EbrIOFile.cpp @@ -19,10 +19,9 @@ #include - std::shared_ptr EbrIOFile::CreateInstance(const char* path, int mode, int share, int pmode) { int fid{}; - int result = _sopen_s(&fid, CPathMapper(path), mode, share, pmode); + int result = _wsopen_s(&fid, CPathMapper(path), mode, share, pmode); if (result != 0) { return nullptr; diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp new file mode 100644 index 0000000000..ce6a4a2b09 --- /dev/null +++ b/Frameworks/Starboard/PathMapper.cpp @@ -0,0 +1,136 @@ +//****************************************************************************** +// +// Copyright (c) Microsoft. All rights reserved. +// +// This code is licensed under the MIT License (MIT). +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//****************************************************************************** + +#include "Starboard.h" +#include "ErrorHandling.h" +#include "StringHelpers.h" + +#include +#include +#include + +#include "Platform/EbrPlatform.h" +#include "PathMapper.h" + +// utility function to tokenize string using delmiters +// d:\src/winobjc ==> d:, src, winobjc +// /src/winobjc ==> "", src, winobjc +// / ==> "" +// src ==> "src" + +static const std::wstring c_currentDir(L"."); +static const std::wstring c_UpDir(L".."); + +static std::list _TokenizeString(const std::wstring& str, const wchar_t* delims) { + std::list components; + + std::size_t start = 0; // start from 0 + std::size_t delimPos = str.find_first_of(delims); + + while (start != std::wstring::npos) { + components.emplace_back(str.substr(start, delimPos - start)); + start = str.find_first_not_of(delims, delimPos); + delimPos = str.find_first_of(delims, start); + } + return components; +} + +// converts any ? to +, we may add more later if needed for compat + +static void _EscapeIllegalPathCharacters(std::wstring& str) { + std::size_t pos = str.find_first_of(L'?'); + while (pos != std::wstring::npos) { + str[pos] = L'+'; + pos = str.find_first_of(L'?', pos + 1); + } +} + +// normalize relative path +// 1 remove any "." or "" components +// 2 from reverse, remove any ".." and preceeding component +// if components becomes empty, return "." + +static void _NormalizeRelativePathComponents(std::list& components) { + for (auto it = components.begin(); it != components.end();) { + if (*it == c_currentDir || it->empty()) { + it = components.erase(it); + continue; + } else if (*it == c_UpDir) { + // remove previous component if it exists + if (it == components.begin()) { + // error, no previous directory exists + components.clear(); + break; + } + // we have to do this in case the last component is .. + it = components.erase(--it); + it = components.erase(it); + } else { + ++it; + } + } + + if (components.empty()) { + components.emplace_back(c_currentDir); + } +} + +// the first component could have special meaning, we map it here +static const std::wstring c_specialFolders[] = { L"Documents", L"cache", L"Library", L"AppSupport", L"tmp", L"shared" }; + +std::wstring _MapPathRoot(const std::wstring& root) { + if (root == c_currentDir) { + return root; + } else if (root == L"~") { + return std::wstring(L"home"); + } + + std::wregex drive(L"[a-zA-Z]:"); + + if (std::regex_match(root, drive)) { + return root; + } + + for (int i = 0; i < _countof(c_specialFolders); ++i) { + if (_wcsicmp(root.c_str(), c_specialFolders[i].c_str()) == 0) { + return Strings::NarrowToWide(EbrGetWritableFolder()) + std::wstring(L"\\") + c_specialFolders[i]; + } + } + + return std::wstring(L".\\") + root; +} + +std::wstring _PathFromComponents(const std::list& components) { + std::wstring path = components.front(); + + auto it = components.begin(); + + std::for_each(++it, components.end(), [&path](const std::wstring& comp) { + path += std::wstring(L"\\"); + path += comp; + }); + _EscapeIllegalPathCharacters(path); + return path; +} + +CPathMapper::CPathMapper(const char* path) { + std::wstring wpath = Strings::NarrowToWide(path); + std::list components = _TokenizeString(wpath, L"/\\"); + + _NormalizeRelativePathComponents(components); + components.front() = _MapPathRoot(components.front()); + mappedPath = _PathFromComponents(components); +} diff --git a/Frameworks/Starboard/PathMapper.h b/Frameworks/Starboard/PathMapper.h index 4135014b60..05fbe45ca6 100644 --- a/Frameworks/Starboard/PathMapper.h +++ b/Frameworks/Starboard/PathMapper.h @@ -14,17 +14,43 @@ // //****************************************************************************** +// +// CPathMapper is a convenience class used to perform some basic path mapping. +// +// 1. construct with a path with either leading or backward slashes (converted internally to / by appendPath) +// appendPath also mysteriously converts ~ to home. This isn't quite helping anyone since there is no 'home' directory on windows. +// we should simply drop this construct. +// 2. fixes up any relative paths +// a) /src/../src ==> /src +// b) /./src/winobjc/../ ==> /src +// c) see fixPath tests in pathmappertests.mm for other supported scenarios (and bugs) +// 3. The fixed up relative path is passed to convert path +// a) -> calls EbrGetRootMapping to replace special directories (ex: Documents, etc). See pathmappertests.mm. +// b) replaces / with \ +// c) removes any leading / introduced by fixPath +// 4. fixedPath is almost never used as real data, except in EbrOpenDir(), where its usage seems to be a bug. +// +// 5. mappedPath is the only real thing needed, so that is all we need to expose. +// +// + class CPathMapper { public: - char mappedPath[4096]; - char fixedPath[4096]; - bool fixedValid, mappedValid; - - char* FixedPath(); - char* MappedPath(); + const std::wstring& MappedPath() const { + return mappedPath; + } + // constructor intentionally takes in char*, assumes UTF8 encoding. CPathMapper(const char* path); - operator const char*() { - return mappedPath; + + operator const wchar_t*() const { + return mappedPath.c_str(); } + + operator bool() const { + return !mappedPath.empty(); + } + +private: + std::wstring mappedPath; }; diff --git a/Frameworks/include/Platform/EbrPlatform.h b/Frameworks/include/Platform/EbrPlatform.h index 0920a781e4..960247c38e 100644 --- a/Frameworks/include/Platform/EbrPlatform.h +++ b/Frameworks/include/Platform/EbrPlatform.h @@ -82,8 +82,6 @@ SB_EXPORT int EbrGetTimeOfDay(struct EbrTimeval* curtime); SB_EXPORT double EbrGetMediaTime(); SB_EXPORT int EbrGetWantedOrientation(); -// maxLen should be MAX_PATH. Sorry Jordan. -SB_EXPORT bool EbrGetRootMapping(const char* dirName, char* dirOut, uint32_t maxLen); SB_EXPORT const char* EbrGetWritableFolder(); SB_EXPORT void EbrSetWritableFolder(const char* folder); diff --git a/build/Starboard/dll/Starboard.def b/build/Starboard/dll/Starboard.def index 521c731e67..7503618360 100644 --- a/build/Starboard/dll/Starboard.def +++ b/build/Starboard/dll/Starboard.def @@ -164,7 +164,6 @@ LIBRARY Starboard EbrGetTimeOfDay EbrGetMediaTime EbrGetWantedOrientation - EbrGetRootMapping EbrGetWritableFolder EbrSetWritableFolder EbrBlockIfBackground diff --git a/build/Starboard/lib/StarboardLib.vcxproj b/build/Starboard/lib/StarboardLib.vcxproj index a08c4fca61..a03e0c0aa2 100644 --- a/build/Starboard/lib/StarboardLib.vcxproj +++ b/build/Starboard/lib/StarboardLib.vcxproj @@ -22,7 +22,6 @@ - @@ -52,6 +51,8 @@ + + {2A00FC26-2ECF-4DF7-8ECF-2D18C5AC61C9} diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm index ea0a64e7ea..9a93663415 100644 --- a/tests/unittests/Starboard/PathMapperTests.mm +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -17,121 +17,83 @@ #include #include "pathmapper.h" -bool fixPath(char* outPath, const char* relativePath); -bool convertPath(char* filePath, const char* relativePath); - -TEST(PathMapper, fixPathNegativeTests) { - char outPath[4096]; - EXPECT_FALSE(fixPath(outPath, "must start with /")); - EXPECT_EQ_MSG(outPath[0], 0, outPath); - - EXPECT_FALSE(fixPath(outPath, "/..")); - EXPECT_EQ_MSG(outPath[0], 0, outPath); - - EXPECT_FALSE(fixPath(outPath, "/./..")); - EXPECT_EQ_MSG(outPath[0], 0, outPath); - - EXPECT_FALSE(fixPath(outPath, "/./src/../../..")); - // this test was failing before, it is a bug - EXPECT_EQ_MSG(outPath[0], 0, outPath); - - EXPECT_FALSE(fixPath(outPath, "/src/../..")); - // this test was failing before, it is a bug - EXPECT_EQ_MSG(outPath[0], 0, outPath); -} - -TEST(PathMapper, fixPathPositiveTests) { - char outPath[4096]; - EXPECT_TRUE(fixPath(outPath, "/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); - - EXPECT_TRUE(fixPath(outPath, "//src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); - - EXPECT_TRUE(fixPath(outPath, "///src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); - - EXPECT_TRUE(fixPath(outPath, "/./src")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); +extern "C" void EbrSetWritableFolder(const char*); - EXPECT_TRUE(fixPath(outPath, "/d:/src")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/d:/src"), outPath); +TEST(PathMapper, pathMapper) { + EbrSetWritableFolder("d:\\temp"); - EXPECT_TRUE(fixPath(outPath, "/./src/../src/")); - // fails, returns //src - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + CPathMapper mapper1("c:/users/winobjc-bot"); + EXPECT_EQ_MSG(0, wcscmp(mapper1, L"c:\\users\\winobjc-bot"), "%S", (const wchar_t*)mapper1); - EXPECT_TRUE(fixPath(outPath, "/./src/winobjc/..")); - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); + CPathMapper mapper2("~/winobjc-bot"); + EXPECT_EQ_MSG(0, wcscmp(mapper2, L".\\home\\winobjc-bot"), "%S", (const wchar_t*)mapper2); - EXPECT_TRUE(fixPath(outPath, "/./././src/winobjc/../winobjc/.././../src/winobjc/..")); - // fails, returns //src - EXPECT_EQ_MSG(0, strcmp(outPath, "/src"), outPath); -} + CPathMapper mapper3("Documents\\mydocuments/subfolder/.././"); + EXPECT_EQ_MSG(0, wcscmp(mapper3, L"d:\\temp\\Documents\\mydocuments"), "%S", (const wchar_t*)mapper3); -extern "C" void EbrSetWritableFolder(const char*); + CPathMapper mapper4(""); + EXPECT_EQ_MSG(0, wcscmp(mapper4, L"."), "%S", (const wchar_t*)mapper4); -TEST(PathMapper, mapPathPositiveTests) { - EbrSetWritableFolder("d:\\temp"); + CPathMapper mapper5("src"); + EXPECT_EQ_MSG(0, wcscmp(mapper5, L".\\src"), "%S", (const wchar_t*)mapper5); - char outPath[4096]; - EXPECT_TRUE(convertPath(outPath, "/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, ".\\src"), outPath); + CPathMapper mapper6("c:/src/"); + EXPECT_EQ_MSG(0, wcscmp(mapper6, L"c:\\src"), "%S", (const wchar_t*)mapper6); - EXPECT_TRUE(convertPath(outPath, "")); - EXPECT_EQ_MSG(0, strcmp(outPath, "."), outPath); + CPathMapper mapper7("/c:/src/"); + EXPECT_EQ_MSG(0, wcscmp(mapper7, L"c:\\src"), "%S", (const wchar_t*)mapper7); - EXPECT_TRUE(convertPath(outPath, "src")); - EXPECT_EQ_MSG(0, strcmp(outPath, ".\\src"), outPath); + CPathMapper mapper8("X:/src/"); + EXPECT_EQ_MSG(0, wcscmp(mapper8, L"X:\\src"), "%S", (const wchar_t*)mapper8); - EXPECT_TRUE(convertPath(outPath, "c:/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "c:\\src"), outPath); + CPathMapper mapper9("/Documents/src/"); + EXPECT_EQ_MSG(0, wcscmp(mapper9, L"d:\\temp\\Documents\\src"), "%S", (const wchar_t*)mapper9); - EXPECT_TRUE(convertPath(outPath, "/c:/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "c:\\src"), outPath); + CPathMapper mapper10("/Documents/./"); + EXPECT_EQ_MSG(0, wcscmp(mapper10, L"d:\\temp\\Documents"), "%S", (const wchar_t*)mapper10); - EXPECT_TRUE(convertPath(outPath, "/X:/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "X:\\src"), outPath); + CPathMapper mapper11("/Cache/test/."); + EXPECT_EQ_MSG(0, wcscmp(mapper11, L"d:\\temp\\cache\\test"), "%S", (const wchar_t*)mapper11); - EXPECT_TRUE(convertPath(outPath, "/Documents/src/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Documents\\src"), outPath); + CPathMapper mapper12("/library"); + EXPECT_EQ_MSG(0, wcscmp(mapper12, L"d:\\temp\\Library"), "%S", (const wchar_t*)mapper12); - EXPECT_TRUE(convertPath(outPath, "/Documents/./")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Documents"), outPath); + CPathMapper mapper13("/AppSupport/././"); + EXPECT_EQ_MSG(0, wcscmp(mapper13, L"d:\\temp\\AppSupport"), "%S", (const wchar_t*)mapper13); - EXPECT_TRUE(convertPath(outPath, "/Cache/test/.")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\cache\\test"), outPath); + CPathMapper mapper14("tmp"); + EXPECT_EQ_MSG(0, wcscmp(mapper14, L"d:\\temp\\tmp"), "%S", (const wchar_t*)mapper14); - EXPECT_TRUE(convertPath(outPath, "/library")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\Library"), outPath); + CPathMapper mapper15("/shared"); + EXPECT_EQ_MSG(0, wcscmp(mapper15, L"d:\\temp\\shared"), "%S", (const wchar_t*)mapper15); - EXPECT_TRUE(convertPath(outPath, "/AppSupport/././")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\AppSupport"), outPath); + CPathMapper mapper16("/AppSupport/.\\?/"); + EXPECT_EQ_MSG(0, wcscmp(mapper16, L"d:\\temp\\AppSupport\\+"), "%S", (const wchar_t*)mapper16); +} - EXPECT_TRUE(convertPath(outPath, "/tmp")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\tmp"), outPath); +TEST(PathMapper, RelativePathTests) { + CPathMapper mapper1("/.."); + EXPECT_EQ_MSG(0, wcscmp(mapper1, L"."), "%S", (const wchar_t*)mapper1); - EXPECT_TRUE(convertPath(outPath, "/shared")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\shared"), outPath); + CPathMapper mapper2("/./.."); + EXPECT_EQ_MSG(0, wcscmp(mapper2, L"."), "%S", (const wchar_t*)mapper2); - EXPECT_TRUE(convertPath(outPath, "/AppSupport/.\\?/")); - EXPECT_EQ_MSG(0, strcmp(outPath, "d:\\temp\\AppSupport\\+"), outPath); -} + CPathMapper mapper3("/./src/../../.."); + // this test was failing before, it is a bug + EXPECT_EQ_MSG(0, wcscmp(mapper3, L"."), "%S", (const wchar_t*)mapper3); -TEST(PathMapper, mapPathNegativeTests) { - char outPath[4096]; + CPathMapper mapper4("/src/../.."); + // this test was failing before, it is a bug + EXPECT_EQ_MSG(0, wcscmp(mapper4, L"."), "%S", (const wchar_t*)mapper4); - // first slash must be / slash, is this just an artifact? - EXPECT_TRUE(convertPath(outPath, "\\AppSupport\\.\\?/")); - // fails previously - EXPECT_EQ_MSG(0, strcmp(outPath, ".\\AppSupport\\+"), outPath); -} + CPathMapper mapper5("/./src/../src/"); + // fails, returns //src + EXPECT_EQ_MSG(0, wcscmp(mapper5, L".\\src"), "%S", (const wchar_t*)mapper5); -TEST(PathMapper, pathMapper) { - CPathMapper mapper1("c:/users/winobjc-bot"); - EXPECT_EQ_MSG(0, strcmp(mapper1, "c:\\users\\winobjc-bot"), (const char*)mapper1); - EXPECT_EQ_MSG(0, strcmp(mapper1.fixedPath, "/c:/users/winobjc-bot"), (const char*)mapper1.fixedPath); + CPathMapper mapper6("/./src/winobjc/.."); + EXPECT_EQ_MSG(0, wcscmp(mapper6, L".\\src"), "%S", (const wchar_t*)mapper6); - CPathMapper mapper2("~/winobjc-bot"); - EXPECT_EQ_MSG(0, strcmp(mapper2, ".\\home\\winobjc-bot"), (const char*)mapper2); + CPathMapper mapper7("/./././src/winobjc/../winobjc/.././../src/winobjc/.."); + // fails, returns //src + EXPECT_EQ_MSG(0, wcscmp(mapper7, L".\\src"), "%S", (const wchar_t*)mapper7); } From e197a9c1395ce0ca32a07176d7d516fa9f2a5572 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Tue, 31 Jan 2017 16:41:40 -0800 Subject: [PATCH 3/8] EbrStat is essentially EbrStat6432i on our platform. Fixed that (thanks NSFileManager unit tests). Also PathMapper properly supports ~ again. --- Frameworks/Starboard/AssetFile.cpp | 7 +++++++ Frameworks/Starboard/PathMapper.cpp | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Frameworks/Starboard/AssetFile.cpp b/Frameworks/Starboard/AssetFile.cpp index ed470fd077..dcce086cec 100644 --- a/Frameworks/Starboard/AssetFile.cpp +++ b/Frameworks/Starboard/AssetFile.cpp @@ -123,6 +123,7 @@ bool EbrIsDir(const char* path) { } int EbrStat(const char* filename, struct stat* ret) { +#ifdef _USE_32BIT_TIME_T CPathMapper map(filename); if (!map) { TraceError(TAG, L"EbrStat failure!"); @@ -137,6 +138,12 @@ int EbrStat(const char* filename, struct stat* ret) { } return _wstat32(map, reinterpret_cast(ret)); +#else + // This is from stat.h, unfortunately, it doesn't define the wstat apis for non 32-bit time_t + // essentially on our platform, EbrStat is same as EbrStat64i32. + // keeping this #ifdef just in case. + return EbrStat64i32(filename, reinterpret_cast(ret)); +#endif } int EbrStat64i32(const char* filename, struct _stat64i32* ret) { diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp index ce6a4a2b09..ea759b8bd0 100644 --- a/Frameworks/Starboard/PathMapper.cpp +++ b/Frameworks/Starboard/PathMapper.cpp @@ -95,7 +95,7 @@ std::wstring _MapPathRoot(const std::wstring& root) { if (root == c_currentDir) { return root; } else if (root == L"~") { - return std::wstring(L"home"); + return std::wstring(L".\\home"); } std::wregex drive(L"[a-zA-Z]:"); From 387b48b11fe9d2d3e08ddff2cdb9c8cc6a996db2 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Tue, 31 Jan 2017 18:45:45 -0800 Subject: [PATCH 4/8] Add NSFileManager test to use special characters. --- tests/unittests/Foundation/NSFileManagerTests.mm | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/unittests/Foundation/NSFileManagerTests.mm b/tests/unittests/Foundation/NSFileManagerTests.mm index 4e161be673..cb66ec1344 100644 --- a/tests/unittests/Foundation/NSFileManagerTests.mm +++ b/tests/unittests/Foundation/NSFileManagerTests.mm @@ -1,4 +1,4 @@ -//****************************************************************************** +//****************************************************************************** // // Copyright (c) Microsoft. All rights reserved. // @@ -213,3 +213,14 @@ // Verify data. ASSERT_OBJCEQ([content dataUsingEncoding:NSUTF8StringEncoding], [NSData dataWithContentsOfURL:destURL]); } + +TEST(NSFileManager, DirectoryWithUTF16Chars) { + wchar_t* specialFolder = L"oÖo winobjc"; + NSString* directoryName = [NSString stringWithCharacters:(const unichar*)specialFolder length:wcslen(specialFolder) * sizeof(wchar_t)]; + NSString* testPath = getPathToFile(directoryName); + + EXPECT_TRUE([[NSFileManager defaultManager] createDirectoryAtPath:testPath attributes:nil]); + EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:testPath]); + EXPECT_TRUE([[NSFileManager defaultManager] removeItemAtPath:testPath error:nil]); + EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:testPath]); +} From 75703577bea89ddcd24c92f930d0a2ed648cfde5 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Wed, 1 Feb 2017 12:58:53 -0800 Subject: [PATCH 5/8] Change EbrGet/SetWritableFolder to take UTF16 paths --- Frameworks/Foundation/NSPathUtilities.mm | 4 ++-- Frameworks/Starboard/EbrFile.cpp | 12 ++++++++---- Frameworks/Starboard/PathMapper.cpp | 2 +- .../UIKit/StarboardXaml/ApplicationCompositor.cpp | 7 +++---- Frameworks/include/Platform/EbrPlatform.h | 4 ++-- tests/unittests/Starboard/PathMapperTests.mm | 8 ++++++-- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/Frameworks/Foundation/NSPathUtilities.mm b/Frameworks/Foundation/NSPathUtilities.mm index 63617f514d..523ced9af6 100644 --- a/Frameworks/Foundation/NSPathUtilities.mm +++ b/Frameworks/Foundation/NSPathUtilities.mm @@ -39,7 +39,7 @@ // Helper that gets the path for a folder dirName under the current app's AppData/Local... directory, // creating the folder if necessary NSString* _getCreateAppDataLocalDir(const char* dirName) { - auto ret = [NSString stringWithFormat:@"%hs/%hs", EbrGetWritableFolder(), dirName]; + auto ret = [NSString stringWithFormat:@"%S/%hs", EbrGetWritableFolder(), dirName]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } @@ -47,7 +47,7 @@ // Override for when a higher-level directory needs to be created first (eg: Foo1/Foo2/) NSString* _getCreateAppDataLocalDir(const char* dirName1, const char* dirName2) { _getCreateAppDataLocalDir(dirName1); - auto ret = [NSString stringWithFormat:@"%hs/%hs/%hs", EbrGetWritableFolder(), dirName1, dirName2]; + auto ret = [NSString stringWithFormat:@"%S/%hs/%hs", EbrGetWritableFolder(), dirName1, dirName2]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index ddbd81a0f0..c4fad1e961 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -161,13 +161,17 @@ bool EbrUnlink(const char* path) { } #define mkdir _mkdir -char g_WritableFolder[2048] = "."; +std::wstring g_WritableFolder(L"."); -void EbrSetWritableFolder(const char* folder) { - strcpy_s(g_WritableFolder, folder); +void EbrSetWritableFolder(const wchar_t* folder) { + g_WritableFolder = folder; } -const char* EbrGetWritableFolder() { +const wchar_t* EbrGetWritableFolder() { + return g_WritableFolder.c_str(); +} + +const std::wstring& _EbrGetWritableFolder() { return g_WritableFolder; } diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp index ea759b8bd0..a944b5e3a5 100644 --- a/Frameworks/Starboard/PathMapper.cpp +++ b/Frameworks/Starboard/PathMapper.cpp @@ -106,7 +106,7 @@ std::wstring _MapPathRoot(const std::wstring& root) { for (int i = 0; i < _countof(c_specialFolders); ++i) { if (_wcsicmp(root.c_str(), c_specialFolders[i].c_str()) == 0) { - return Strings::NarrowToWide(EbrGetWritableFolder()) + std::wstring(L"\\") + c_specialFolders[i]; + return EbrGetWritableFolder() + std::wstring(L"\\") + c_specialFolders[i]; } } diff --git a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp index 0860d1cd58..e49eec18a5 100644 --- a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp +++ b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp @@ -39,12 +39,11 @@ void InitializeApp() { initialized = true; // Set our writable and temp folders - char writableFolder[2048]; - size_t outLen; auto pathData = Windows::Storage::ApplicationData::Current->LocalFolder->Path; - wcstombs_s(&outLen, writableFolder, pathData->Data(), sizeof(writableFolder) - 1); - EbrSetWritableFolder(writableFolder); + EbrSetWritableFolder(pathData->Data()); + char writableFolder[2048]; + size_t outLen; auto tempPathData = Windows::Storage::ApplicationData::Current->TemporaryFolder->Path; wcstombs_s(&outLen, writableFolder, tempPathData->Data(), sizeof(writableFolder) - 1); SetTemporaryFolder(writableFolder); diff --git a/Frameworks/include/Platform/EbrPlatform.h b/Frameworks/include/Platform/EbrPlatform.h index 960247c38e..94196f44f0 100644 --- a/Frameworks/include/Platform/EbrPlatform.h +++ b/Frameworks/include/Platform/EbrPlatform.h @@ -82,8 +82,8 @@ SB_EXPORT int EbrGetTimeOfDay(struct EbrTimeval* curtime); SB_EXPORT double EbrGetMediaTime(); SB_EXPORT int EbrGetWantedOrientation(); -SB_EXPORT const char* EbrGetWritableFolder(); -SB_EXPORT void EbrSetWritableFolder(const char* folder); +SB_EXPORT const wchar_t* EbrGetWritableFolder(); +SB_EXPORT void EbrSetWritableFolder(const wchar_t* folder); SB_EXPORT void EbrBlockIfBackground(); diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm index 9a93663415..daf88fc927 100644 --- a/tests/unittests/Starboard/PathMapperTests.mm +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -17,10 +17,12 @@ #include #include "pathmapper.h" -extern "C" void EbrSetWritableFolder(const char*); +extern "C" void EbrSetWritableFolder(const wchar_t*); +extern "C" const wchar_t* EbrGetWritableFolder(); TEST(PathMapper, pathMapper) { - EbrSetWritableFolder("d:\\temp"); + std::wstring writableFolder = EbrGetWritableFolder(); + EbrSetWritableFolder(L"d:\\temp"); CPathMapper mapper1("c:/users/winobjc-bot"); EXPECT_EQ_MSG(0, wcscmp(mapper1, L"c:\\users\\winobjc-bot"), "%S", (const wchar_t*)mapper1); @@ -69,6 +71,8 @@ CPathMapper mapper16("/AppSupport/.\\?/"); EXPECT_EQ_MSG(0, wcscmp(mapper16, L"d:\\temp\\AppSupport\\+"), "%S", (const wchar_t*)mapper16); + + EbrSetWritableFolder(writableFolder.c_str()); } TEST(PathMapper, RelativePathTests) { From 210e10fe70d033b5d4e6c3d351b4cbd0e167235d Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Wed, 1 Feb 2017 16:06:14 -0800 Subject: [PATCH 6/8] EbrGet/SetWritablePath are now IwGet/SetWritablePath, and support UTF-16. Address CR feedback. --- Frameworks/Starboard/AssetFile.cpp | 3 ++- Frameworks/Starboard/EbrFile.cpp | 6 +++--- Frameworks/Starboard/PathMapper.cpp | 19 +++++++++---------- .../StarboardXaml/ApplicationCompositor.cpp | 2 +- Frameworks/include/Platform/EbrPlatform.h | 4 ++-- build/Starboard/dll/Starboard.def | 5 +++-- tests/unittests/Starboard/PathMapperTests.mm | 8 ++++---- 7 files changed, 24 insertions(+), 23 deletions(-) diff --git a/Frameworks/Starboard/AssetFile.cpp b/Frameworks/Starboard/AssetFile.cpp index dcce086cec..5c05f730e2 100644 --- a/Frameworks/Starboard/AssetFile.cpp +++ b/Frameworks/Starboard/AssetFile.cpp @@ -79,8 +79,9 @@ bool EbrFSDirReader::readNext(EbrDir* curDir, EbrDirEnt* ent) { EbrDir* EbrOpenDir(const char* path) { CPathMapper map(path); - if (!map) + if (!map) { return NULL; + } EbrDirReader* fsReader = EbrFSDirReader::open(map.MappedPath()); if (!fsReader) { diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index c4fad1e961..97daa181a2 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -163,15 +163,15 @@ bool EbrUnlink(const char* path) { #define mkdir _mkdir std::wstring g_WritableFolder(L"."); -void EbrSetWritableFolder(const wchar_t* folder) { +void IwSetWritableFolder(const wchar_t* folder) { g_WritableFolder = folder; } -const wchar_t* EbrGetWritableFolder() { +const wchar_t* IwGetWritableFolder() { return g_WritableFolder.c_str(); } -const std::wstring& _EbrGetWritableFolder() { +const std::wstring& _IwGetWritableFolder() { return g_WritableFolder; } diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp index a944b5e3a5..790f84110e 100644 --- a/Frameworks/Starboard/PathMapper.cpp +++ b/Frameworks/Starboard/PathMapper.cpp @@ -20,12 +20,11 @@ #include #include -#include #include "Platform/EbrPlatform.h" #include "PathMapper.h" -// utility function to tokenize string using delmiters +// utility function to tokenize string using delimiters // d:\src/winobjc ==> d:, src, winobjc // /src/winobjc ==> "", src, winobjc // / ==> "" @@ -59,9 +58,9 @@ static void _EscapeIllegalPathCharacters(std::wstring& str) { } // normalize relative path -// 1 remove any "." or "" components -// 2 from reverse, remove any ".." and preceeding component -// if components becomes empty, return "." +// 1 remove any "." or "" components +// 2 remove any ".." and preceeding component +// 3 if components becomes empty, return "." static void _NormalizeRelativePathComponents(std::list& components) { for (auto it = components.begin(); it != components.end();) { @@ -76,6 +75,8 @@ static void _NormalizeRelativePathComponents(std::list& components break; } // we have to do this in case the last component is .. + // erase the previous component first because erase returns the next item + // which would be "..". it = components.erase(--it); it = components.erase(it); } else { @@ -98,15 +99,13 @@ std::wstring _MapPathRoot(const std::wstring& root) { return std::wstring(L".\\home"); } - std::wregex drive(L"[a-zA-Z]:"); - - if (std::regex_match(root, drive)) { + if (root.length() == 2 && iswalpha(root[0]) && root[1] == L':') { return root; } - for (int i = 0; i < _countof(c_specialFolders); ++i) { + for (auto root : c_specialFolders) { if (_wcsicmp(root.c_str(), c_specialFolders[i].c_str()) == 0) { - return EbrGetWritableFolder() + std::wstring(L"\\") + c_specialFolders[i]; + return IwGetWritableFolder() + std::wstring(L"\\") + c_specialFolders[i]; } } diff --git a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp index e49eec18a5..94747380a3 100644 --- a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp +++ b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp @@ -40,7 +40,7 @@ void InitializeApp() { // Set our writable and temp folders auto pathData = Windows::Storage::ApplicationData::Current->LocalFolder->Path; - EbrSetWritableFolder(pathData->Data()); + IwSetWritableFolder(pathData->Data()); char writableFolder[2048]; size_t outLen; diff --git a/Frameworks/include/Platform/EbrPlatform.h b/Frameworks/include/Platform/EbrPlatform.h index 94196f44f0..57d8d8b60c 100644 --- a/Frameworks/include/Platform/EbrPlatform.h +++ b/Frameworks/include/Platform/EbrPlatform.h @@ -82,8 +82,8 @@ SB_EXPORT int EbrGetTimeOfDay(struct EbrTimeval* curtime); SB_EXPORT double EbrGetMediaTime(); SB_EXPORT int EbrGetWantedOrientation(); -SB_EXPORT const wchar_t* EbrGetWritableFolder(); -SB_EXPORT void EbrSetWritableFolder(const wchar_t* folder); +SB_EXPORT const wchar_t* IwGetWritableFolder(); +SB_EXPORT void IwSetWritableFolder(const wchar_t* folder); SB_EXPORT void EbrBlockIfBackground(); diff --git a/build/Starboard/dll/Starboard.def b/build/Starboard/dll/Starboard.def index 7503618360..3b8023c58d 100644 --- a/build/Starboard/dll/Starboard.def +++ b/build/Starboard/dll/Starboard.def @@ -164,8 +164,6 @@ LIBRARY Starboard EbrGetTimeOfDay EbrGetMediaTime EbrGetWantedOrientation - EbrGetWritableFolder - EbrSetWritableFolder EbrBlockIfBackground EbrEventInit EbrEventSignal @@ -175,5 +173,8 @@ LIBRARY Starboard EbrEventTimedMultipleWait EbrEventDestroy + IwGetWritableFolder + IwSetWritableFolder + ; REMOVE / CLEANUP C++ EXPORTS ?format@string@woc@@YA?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@PBDZZ diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm index daf88fc927..8a17f62eea 100644 --- a/tests/unittests/Starboard/PathMapperTests.mm +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -17,12 +17,12 @@ #include #include "pathmapper.h" -extern "C" void EbrSetWritableFolder(const wchar_t*); -extern "C" const wchar_t* EbrGetWritableFolder(); +extern "C" void IwSetWritableFolder(const wchar_t*); +extern "C" const wchar_t* IwGetWritableFolder(); TEST(PathMapper, pathMapper) { std::wstring writableFolder = EbrGetWritableFolder(); - EbrSetWritableFolder(L"d:\\temp"); + IwSetWritableFolder(L"d:\\temp"); CPathMapper mapper1("c:/users/winobjc-bot"); EXPECT_EQ_MSG(0, wcscmp(mapper1, L"c:\\users\\winobjc-bot"), "%S", (const wchar_t*)mapper1); @@ -72,7 +72,7 @@ CPathMapper mapper16("/AppSupport/.\\?/"); EXPECT_EQ_MSG(0, wcscmp(mapper16, L"d:\\temp\\AppSupport\\+"), "%S", (const wchar_t*)mapper16); - EbrSetWritableFolder(writableFolder.c_str()); + IwSetWritableFolder(writableFolder.c_str()); } TEST(PathMapper, RelativePathTests) { From 1b5359a6b324c56b19528392c8ac644966b1e935 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Wed, 1 Feb 2017 17:03:50 -0800 Subject: [PATCH 7/8] CR feedback and some missed code changes --- Frameworks/Foundation/NSPathUtilities.mm | 4 +- Frameworks/Starboard/PathMapper.cpp | 6 +-- tests/unittests/Starboard/PathMapperTests.mm | 48 ++++++++++---------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Frameworks/Foundation/NSPathUtilities.mm b/Frameworks/Foundation/NSPathUtilities.mm index 523ced9af6..0349eea3b7 100644 --- a/Frameworks/Foundation/NSPathUtilities.mm +++ b/Frameworks/Foundation/NSPathUtilities.mm @@ -39,7 +39,7 @@ // Helper that gets the path for a folder dirName under the current app's AppData/Local... directory, // creating the folder if necessary NSString* _getCreateAppDataLocalDir(const char* dirName) { - auto ret = [NSString stringWithFormat:@"%S/%hs", EbrGetWritableFolder(), dirName]; + auto ret = [NSString stringWithFormat:@"%S/%hs", IwGetWritableFolder(), dirName]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } @@ -47,7 +47,7 @@ // Override for when a higher-level directory needs to be created first (eg: Foo1/Foo2/) NSString* _getCreateAppDataLocalDir(const char* dirName1, const char* dirName2) { _getCreateAppDataLocalDir(dirName1); - auto ret = [NSString stringWithFormat:@"%S/%hs/%hs", EbrGetWritableFolder(), dirName1, dirName2]; + auto ret = [NSString stringWithFormat:@"%S/%hs/%hs", IwGetWritableFolder(), dirName1, dirName2]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp index 790f84110e..9dc004604c 100644 --- a/Frameworks/Starboard/PathMapper.cpp +++ b/Frameworks/Starboard/PathMapper.cpp @@ -103,9 +103,9 @@ std::wstring _MapPathRoot(const std::wstring& root) { return root; } - for (auto root : c_specialFolders) { - if (_wcsicmp(root.c_str(), c_specialFolders[i].c_str()) == 0) { - return IwGetWritableFolder() + std::wstring(L"\\") + c_specialFolders[i]; + for (auto folder : c_specialFolders) { + if (_wcsicmp(root.c_str(), folder.c_str()) == 0) { + return IwGetWritableFolder() + std::wstring(L"\\") + folder; } } diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm index 8a17f62eea..8b89a78c92 100644 --- a/tests/unittests/Starboard/PathMapperTests.mm +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -21,83 +21,83 @@ extern "C" const wchar_t* IwGetWritableFolder(); TEST(PathMapper, pathMapper) { - std::wstring writableFolder = EbrGetWritableFolder(); + std::wstring writableFolder = IwGetWritableFolder(); IwSetWritableFolder(L"d:\\temp"); CPathMapper mapper1("c:/users/winobjc-bot"); - EXPECT_EQ_MSG(0, wcscmp(mapper1, L"c:\\users\\winobjc-bot"), "%S", (const wchar_t*)mapper1); + EXPECT_STREQ(mapper1, L"c:\\users\\winobjc-bot"); CPathMapper mapper2("~/winobjc-bot"); - EXPECT_EQ_MSG(0, wcscmp(mapper2, L".\\home\\winobjc-bot"), "%S", (const wchar_t*)mapper2); + EXPECT_STREQ(mapper2, L".\\home\\winobjc-bot"); CPathMapper mapper3("Documents\\mydocuments/subfolder/.././"); - EXPECT_EQ_MSG(0, wcscmp(mapper3, L"d:\\temp\\Documents\\mydocuments"), "%S", (const wchar_t*)mapper3); + EXPECT_STREQ(mapper3, L"d:\\temp\\Documents\\mydocuments"); CPathMapper mapper4(""); - EXPECT_EQ_MSG(0, wcscmp(mapper4, L"."), "%S", (const wchar_t*)mapper4); + EXPECT_STREQ(mapper4, L"."); CPathMapper mapper5("src"); - EXPECT_EQ_MSG(0, wcscmp(mapper5, L".\\src"), "%S", (const wchar_t*)mapper5); + EXPECT_STREQ(mapper5, L".\\src"); CPathMapper mapper6("c:/src/"); - EXPECT_EQ_MSG(0, wcscmp(mapper6, L"c:\\src"), "%S", (const wchar_t*)mapper6); + EXPECT_STREQ(mapper6, L"c:\\src"); CPathMapper mapper7("/c:/src/"); - EXPECT_EQ_MSG(0, wcscmp(mapper7, L"c:\\src"), "%S", (const wchar_t*)mapper7); + EXPECT_STREQ(mapper7, L"c:\\src"); CPathMapper mapper8("X:/src/"); - EXPECT_EQ_MSG(0, wcscmp(mapper8, L"X:\\src"), "%S", (const wchar_t*)mapper8); + EXPECT_STREQ(mapper8, L"X:\\src"); CPathMapper mapper9("/Documents/src/"); - EXPECT_EQ_MSG(0, wcscmp(mapper9, L"d:\\temp\\Documents\\src"), "%S", (const wchar_t*)mapper9); + EXPECT_STREQ(mapper9, L"d:\\temp\\Documents\\src"); CPathMapper mapper10("/Documents/./"); - EXPECT_EQ_MSG(0, wcscmp(mapper10, L"d:\\temp\\Documents"), "%S", (const wchar_t*)mapper10); + EXPECT_STREQ(mapper10, L"d:\\temp\\Documents"); CPathMapper mapper11("/Cache/test/."); - EXPECT_EQ_MSG(0, wcscmp(mapper11, L"d:\\temp\\cache\\test"), "%S", (const wchar_t*)mapper11); + EXPECT_STREQ(mapper11, L"d:\\temp\\cache\\test"); CPathMapper mapper12("/library"); - EXPECT_EQ_MSG(0, wcscmp(mapper12, L"d:\\temp\\Library"), "%S", (const wchar_t*)mapper12); + EXPECT_STREQ(mapper12, L"d:\\temp\\Library"); CPathMapper mapper13("/AppSupport/././"); - EXPECT_EQ_MSG(0, wcscmp(mapper13, L"d:\\temp\\AppSupport"), "%S", (const wchar_t*)mapper13); + EXPECT_STREQ(mapper13, L"d:\\temp\\AppSupport"); CPathMapper mapper14("tmp"); - EXPECT_EQ_MSG(0, wcscmp(mapper14, L"d:\\temp\\tmp"), "%S", (const wchar_t*)mapper14); + EXPECT_STREQ(mapper14, L"d:\\temp\\tmp"); CPathMapper mapper15("/shared"); - EXPECT_EQ_MSG(0, wcscmp(mapper15, L"d:\\temp\\shared"), "%S", (const wchar_t*)mapper15); + EXPECT_STREQ(mapper15, L"d:\\temp\\shared"); CPathMapper mapper16("/AppSupport/.\\?/"); - EXPECT_EQ_MSG(0, wcscmp(mapper16, L"d:\\temp\\AppSupport\\+"), "%S", (const wchar_t*)mapper16); + EXPECT_STREQ(mapper16, L"d:\\temp\\AppSupport\\+"); IwSetWritableFolder(writableFolder.c_str()); } TEST(PathMapper, RelativePathTests) { CPathMapper mapper1("/.."); - EXPECT_EQ_MSG(0, wcscmp(mapper1, L"."), "%S", (const wchar_t*)mapper1); + EXPECT_STREQ(mapper1, L"."); CPathMapper mapper2("/./.."); - EXPECT_EQ_MSG(0, wcscmp(mapper2, L"."), "%S", (const wchar_t*)mapper2); + EXPECT_STREQ(mapper2, L"."); CPathMapper mapper3("/./src/../../.."); // this test was failing before, it is a bug - EXPECT_EQ_MSG(0, wcscmp(mapper3, L"."), "%S", (const wchar_t*)mapper3); + EXPECT_STREQ(mapper3, L"."); CPathMapper mapper4("/src/../.."); // this test was failing before, it is a bug - EXPECT_EQ_MSG(0, wcscmp(mapper4, L"."), "%S", (const wchar_t*)mapper4); + EXPECT_STREQ(mapper4, L"."); CPathMapper mapper5("/./src/../src/"); // fails, returns //src - EXPECT_EQ_MSG(0, wcscmp(mapper5, L".\\src"), "%S", (const wchar_t*)mapper5); + EXPECT_STREQ(mapper5, L".\\src"); CPathMapper mapper6("/./src/winobjc/.."); - EXPECT_EQ_MSG(0, wcscmp(mapper6, L".\\src"), "%S", (const wchar_t*)mapper6); + EXPECT_STREQ(mapper6, L".\\src"); CPathMapper mapper7("/./././src/winobjc/../winobjc/.././../src/winobjc/.."); // fails, returns //src - EXPECT_EQ_MSG(0, wcscmp(mapper7, L".\\src"), "%S", (const wchar_t*)mapper7); + EXPECT_STREQ(mapper7, L".\\src"); } From 8e47f6fec2b238cc681abe4490ae856a6dba176c Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Thu, 2 Feb 2017 07:52:12 -0800 Subject: [PATCH 8/8] Ensure that the default folders are created before use --- Frameworks/Starboard/EbrFile.cpp | 2 ++ Frameworks/Starboard/PathMapper.cpp | 13 +++++++++++++ Frameworks/Starboard/PathMapper.h | 2 ++ 3 files changed, 17 insertions(+) diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index 97daa181a2..7cf78dbc7b 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -165,6 +165,8 @@ std::wstring g_WritableFolder(L"."); void IwSetWritableFolder(const wchar_t* folder) { g_WritableFolder = folder; + // recreate the default folders + CPathMapper::CreateDefaultPaths(); } const wchar_t* IwGetWritableFolder() { diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp index 9dc004604c..2cc424bd3d 100644 --- a/Frameworks/Starboard/PathMapper.cpp +++ b/Frameworks/Starboard/PathMapper.cpp @@ -126,6 +126,9 @@ std::wstring _PathFromComponents(const std::list& components) { } CPathMapper::CPathMapper(const char* path) { + // create default paths once + static bool createDefaultPaths = CreateDefaultPaths(); + std::wstring wpath = Strings::NarrowToWide(path); std::list components = _TokenizeString(wpath, L"/\\"); @@ -133,3 +136,13 @@ CPathMapper::CPathMapper(const char* path) { components.front() = _MapPathRoot(components.front()); mappedPath = _PathFromComponents(components); } + +bool CPathMapper::CreateDefaultPaths() { + for (auto folder : c_specialFolders) { + _wmkdir((IwGetWritableFolder() + std::wstring(L"\\") + folder).c_str()); + } + // also create "Documents\\Library" + _wmkdir((IwGetWritableFolder() + std::wstring(L"\\Documents\\Library")).c_str()); + + return true; +} diff --git a/Frameworks/Starboard/PathMapper.h b/Frameworks/Starboard/PathMapper.h index 05fbe45ca6..c3311e7c7b 100644 --- a/Frameworks/Starboard/PathMapper.h +++ b/Frameworks/Starboard/PathMapper.h @@ -51,6 +51,8 @@ class CPathMapper { return !mappedPath.empty(); } + static bool CreateDefaultPaths(); + private: std::wstring mappedPath; };