From 5b580e4d7aa054719e991dc90c76e0eee8399ec8 Mon Sep 17 00:00:00 2001 From: Yuri Gorshenin Date: Thu, 15 Oct 2015 14:06:01 +0300 Subject: [PATCH] Review fixes. --- base/base_tests/string_utils_test.cpp | 15 ++++++++ base/string_utils.cpp | 9 +++++ base/string_utils.hpp | 2 + platform/local_country_file_utils.cpp | 13 ++----- platform/platform.hpp | 2 + platform/platform_tests/platform_test.cpp | 9 +++++ platform/platform_unix_impl.cpp | 46 ++++++++++++++++++----- platform/platform_win.cpp | 8 +++- 8 files changed, 84 insertions(+), 20 deletions(-) diff --git a/base/base_tests/string_utils_test.cpp b/base/base_tests/string_utils_test.cpp index 615edb7e32..b827a2776f 100644 --- a/base/base_tests/string_utils_test.cpp +++ b/base/base_tests/string_utils_test.cpp @@ -421,6 +421,21 @@ UNIT_TEST(StartsWith) TEST(!StartsWith(s, "axy"), ()); } +UNIT_TEST(EndsWith) +{ + using namespace strings; + TEST(EndsWith(string(), ""), ()); + + string s("xyz"); + TEST(EndsWith(s, ""), ()); + TEST(EndsWith(s, "z"), ()); + TEST(EndsWith(s, "yz"), ()); + TEST(EndsWith(s, "xyz"), ()); + TEST(!EndsWith(s, "abcxyz"), ()); + TEST(!EndsWith(s, "ayz"), ()); + TEST(!EndsWith(s, "axyz"), ()); +} + UNIT_TEST(UniString_LessAndEqualsAndNotEquals) { vector v; diff --git a/base/string_utils.cpp b/base/string_utils.cpp index 2afc7e14e3..0e11c47ae9 100644 --- a/base/string_utils.cpp +++ b/base/string_utils.cpp @@ -170,6 +170,15 @@ bool StartsWith(string const & s1, char const * s2) return (s1.compare(0, strlen(s2), s2) == 0); } +bool EndsWith(string const & s1, char const * s2) +{ + size_t const n = s1.size(); + size_t const m = strlen(s2); + if (n < m) + return false; + return (s1.compare(n - m, m, s2) == 0); +} + string to_string_dac(double d, int dac) { dac = min(numeric_limits::digits10, dac); diff --git a/base/string_utils.hpp b/base/string_utils.hpp index d1f318873b..57d8cf5d9a 100644 --- a/base/string_utils.hpp +++ b/base/string_utils.hpp @@ -262,6 +262,8 @@ string to_string_dac(double d, int dac); bool StartsWith(string const & s1, char const * s2); +bool EndsWith(string const & s1, char const * s2); + /// Try to guess if it's HTML or not. No guarantee. bool IsHTML(string const & utf8); diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index ab7d6f99c4..966d292a69 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -6,6 +6,7 @@ #include "coding/internal/file_data.hpp" #include "coding/reader.hpp" +#include "base/assert.hpp" #include "base/string_utils.hpp" #include "base/logging.hpp" #include "base/regexp.hpp" @@ -27,15 +28,6 @@ char const kOffsetsExt[] = ".offsets"; size_t const kMaxTimestampLength = 18; -bool IsSpecialFile(string const & file) { return file == "." || file == ".."; } - -bool IsEmptyDirectory(string const & directory) -{ - Platform::FilesList files; - Platform::GetFilesByRegExp(directory, ".*", files); - return all_of(files.begin(), files.end(), &IsSpecialFile); -} - bool GetFileTypeChecked(string const & path, Platform::EFileType & type) { Platform::EError const ret = Platform::GetFileType(path, type); @@ -97,6 +89,7 @@ void DeleteDownloaderFilesForCountry(CountryFile const & countryFile, int64_t ve for (MapOptions file : {MapOptions::Map, MapOptions::CarRouting}) { string const path = GetFileDownloadPath(countryFile, file, version); + ASSERT(strings::EndsWith(path, READY_FILE_EXTENSION), ()); my::DeleteFileX(path); my::DeleteFileX(path + RESUME_FILE_EXTENSION); my::DeleteFileX(path + DOWNLOADING_FILE_EXTENSION); @@ -145,7 +138,7 @@ void CleanupMapsDirectory(int64_t latestVersion) DeleteDownloaderFilesForAllCountries(subdirPath); // Remove subdirectory if it does not contain any files except "." and "..". - if (subdir != "." && IsEmptyDirectory(subdirPath)) + if (subdir != "." && Platform::IsDirectoryEmpty(subdirPath)) { Platform::EError const ret = Platform::RmDir(subdirPath); ASSERT_EQUAL(Platform::ERR_OK, ret, diff --git a/platform/platform.hpp b/platform/platform.hpp index d90f3b089d..312196ac3b 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -140,6 +140,8 @@ public: static void GetFilesByType(string const & directory, unsigned typeMask, FilesList & outFiles); + static bool IsDirectoryEmpty(string const & directory); + static EError GetFileType(string const & path, EFileType & type); /// @return false if file is not exist diff --git a/platform/platform_tests/platform_test.cpp b/platform/platform_tests/platform_test.cpp index bfd16357dd..851b48cf9c 100644 --- a/platform/platform_tests/platform_test.cpp +++ b/platform/platform_tests/platform_test.cpp @@ -106,11 +106,20 @@ UNIT_TEST(DirsRoutines) Platform & platform = GetPlatform(); string const baseDir = platform.WritableDir(); string const testDir = my::JoinFoldersToPath(baseDir, "test-dir"); + string const testFile = my::JoinFoldersToPath(testDir, "test-file"); TEST(!Platform::IsFileExistsByFullPath(testDir), ()); TEST_EQUAL(platform.MkDir(testDir), Platform::ERR_OK, ()); TEST(Platform::IsFileExistsByFullPath(testDir), ()); + TEST(Platform::IsDirectoryEmpty(testDir), ()); + + { + FileWriter writer(testFile); + } + TEST(!Platform::IsDirectoryEmpty(testDir), ()); + FileWriter::DeleteFileX(testFile); + TEST_EQUAL(Platform::RmDir(testDir), Platform::ERR_OK, ()); TEST(!Platform::IsFileExistsByFullPath(testDir), ()); diff --git a/platform/platform_unix_impl.cpp b/platform/platform_unix_impl.cpp index 19d12d1b5a..74dac9c5e7 100644 --- a/platform/platform_unix_impl.cpp +++ b/platform/platform_unix_impl.cpp @@ -8,6 +8,8 @@ #include "base/scope_guard.hpp" #include "std/algorithm.hpp" +#include "std/cstring.hpp" +#include "std/unique_ptr.hpp" #include #include @@ -20,6 +22,18 @@ #include #endif +namespace +{ +struct CloseDir +{ + void operator()(DIR * dir) const + { + if (dir) + closedir(dir); + } +}; +} // namespace + void Platform::GetSystemFontNames(FilesList & res) const { #if defined(OMIM_OS_MAC) || defined(OMIM_OS_IPHONE) @@ -136,6 +150,24 @@ bool Platform::IsFileExistsByFullPath(string const & filePath) return stat(filePath.c_str(), &s) == 0; } +bool Platform::IsDirectoryEmpty(string const & directory) +{ + unique_ptr dir(opendir(directory.c_str())); + if (!dir) + return true; + + struct dirent * entry; + + // Invariant: all files met so far are "." or "..". + while ((entry = readdir(dir.get())) != nullptr) + { + // A file is not a special UNIX file. Early exit here. + if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) + return false; + } + return true; +} + bool Platform::GetFileSizeByFullPath(string const & filePath, uint64_t & size) { struct stat s; @@ -168,26 +200,22 @@ Platform::TStorageStatus Platform::GetWritableStorageStatus(uint64_t neededSize) namespace pl { - void EnumerateFilesByRegExp(string const & directory, string const & regexp, vector & res) { - DIR * dir; - struct dirent * entry; - if ((dir = opendir(directory.c_str())) == NULL) + unique_ptr dir(opendir(directory.c_str())); + if (!dir) return; regexp::RegExpT exp; regexp::Create(regexp, exp); - while ((entry = readdir(dir)) != 0) + struct dirent * entry; + while ((entry = readdir(dir.get())) != 0) { string const name(entry->d_name); if (regexp::IsExist(name, exp)) res.push_back(name); } - - closedir(dir); -} - } +} // namespace pl diff --git a/platform/platform_win.cpp b/platform/platform_win.cpp index b69ea1ff89..1bbe9d5df0 100644 --- a/platform/platform_win.cpp +++ b/platform/platform_win.cpp @@ -10,8 +10,9 @@ #include #include -#include +#include #include +#include static bool GetUserWritableDir(string & outDir) { @@ -155,6 +156,11 @@ Platform::TStorageStatus Platform::GetWritableStorageStatus(uint64_t neededSize) return STORAGE_OK; } +bool Platform::IsDirectoryEmpty(string const & directory) +{ + return PathIsDirectoryEmptyA(directory.c_str()); +} + bool Platform::GetFileSizeByFullPath(string const & filePath, uint64_t & size) { HANDLE hFile = CreateFileA(filePath.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);