From 1b146e77bca16c5a9a1585c380d30869e83c57df Mon Sep 17 00:00:00 2001 From: Madhura Jayaraman Date: Fri, 19 Jul 2024 11:31:08 -0700 Subject: [PATCH] Finish cleaning up directory references (#3817) Test-On-Device: true b/302730696 --- starboard/android/shared/android_main.cc | 29 +++++++++-- starboard/common/file.cc | 26 ++++++---- starboard/shared/win32/posix_emu/dirent.cc | 56 ++++++++++++++-------- 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/starboard/android/shared/android_main.cc b/starboard/android/shared/android_main.cc index 4de29227a1d9e..68f2ed249715f 100644 --- a/starboard/android/shared/android_main.cc +++ b/starboard/android/shared/android_main.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include "game-activity/GameActivity.h" @@ -92,15 +93,33 @@ std::string GetStartDeepLink() { #if SB_IS(EVERGREEN_COMPATIBLE) bool CopyDirContents(const std::string& src_dir_path, const std::string& dst_dir_path) { - SbDirectory src_dir = SbDirectoryOpen(src_dir_path.c_str(), NULL); - if (!SbDirectoryIsValid(src_dir)) { + DIR* src_dir = opendir(src_dir_path.c_str()); + if (!src_dir) { SB_LOG(WARNING) << "Failed to open dir=" << src_dir_path; return false; } std::vector filename_buffer(kSbFileMaxName); - while (SbDirectoryGetNext(src_dir, filename_buffer.data(), - filename_buffer.size())) { + + while (true) { + if (filename_buffer.size() < kSbFileMaxName) { + break; + } + + if (!src_dir || !filename_buffer.data()) { + break; + } + + struct dirent dirent_buffer; + struct dirent* dirent; + int result = readdir_r(src_dir, &dirent_buffer, &dirent); + if (result || !dirent) { + break; + } + + starboard::strlcpy(filename_buffer.data(), dirent->d_name, + filename_buffer.size()); + std::string filename(filename_buffer.begin(), filename_buffer.end()); std::string path_to_src_file = src_dir_path + kSbFileSepString + filename; SbFile src_file = @@ -151,7 +170,7 @@ bool CopyDirContents(const std::string& src_dir_path, SbFileClose(dst_file); } - SbDirectoryClose(src_dir); + closedir(src_dir); return true; } diff --git a/starboard/common/file.cc b/starboard/common/file.cc index 7729e9971259e..99cb273f9b025 100644 --- a/starboard/common/file.cc +++ b/starboard/common/file.cc @@ -14,6 +14,7 @@ #include "starboard/common/file.h" +#include #include #include #include @@ -24,6 +25,7 @@ #include "starboard/common/log.h" #include "starboard/common/metrics/stats_tracker.h" +#include "starboard/common/string.h" #include "starboard/configuration_constants.h" #include "starboard/directory.h" #include "starboard/file.h" @@ -32,8 +34,8 @@ namespace starboard { namespace { -bool DirectoryCloseLogFailure(const char* path, SbDirectory dir) { - if (!SbDirectoryClose(dir)) { +bool DirectoryCloseLogFailure(const char* path, DIR* dir) { + if (closedir(dir) != 0) { SB_LOG(ERROR) << "Failed to close directory: '" << path << "'"; return false; } @@ -79,13 +81,10 @@ bool SbFileDeleteRecursive(const char* path, bool preserve_root) { return false; } - SbFileError err = kSbFileOk; - SbDirectory dir = kSbDirectoryInvalid; - - dir = SbDirectoryOpen(path, &err); + DIR* dir = opendir(path); // The |path| points to a file. Remove it and return. - if (err != kSbFileOk) { + if (!dir) { return SbFileDelete(path); } @@ -93,7 +92,18 @@ bool SbFileDeleteRecursive(const char* path, bool preserve_root) { std::vector entry(kSbFileMaxName); - while (SbDirectoryGetNext(dir, entry.data(), kSbFileMaxName)) { + struct dirent dirent_buffer; + struct dirent* dirent; + while (true) { + if (entry.size() < kSbFileMaxName || !dir || !entry.data()) { + break; + } + int result = readdir_r(dir, &dirent_buffer, &dirent); + if (result || !dirent) { + break; + } + starboard::strlcpy(entry.data(), dirent->d_name, kSbFileMaxName); + if (!strcmp(entry.data(), ".") || !strcmp(entry.data(), "..")) { continue; } diff --git a/starboard/shared/win32/posix_emu/dirent.cc b/starboard/shared/win32/posix_emu/dirent.cc index 227b367a8dad7..472806d621364 100644 --- a/starboard/shared/win32/posix_emu/dirent.cc +++ b/starboard/shared/win32/posix_emu/dirent.cc @@ -61,9 +61,9 @@ static int handle_db_put(std::deque next_directory_entry) { return fd; } -static std::deque handle_db_get(int handle, bool erase) { +static std::deque handle_db_get(int fd, bool erase) { std::deque empty_deque; - if (handle < 0) { + if (fd < 0) { return empty_deque; } EnterCriticalSection(&g_critical_section.critical_section_); @@ -72,7 +72,7 @@ static std::deque handle_db_get(int handle, bool erase) { return empty_deque; } - auto itr = directory_map->find(handle); + auto itr = directory_map->find(fd); if (itr == directory_map->end()) { LeaveCriticalSection(&g_critical_section.critical_section_); return empty_deque; @@ -92,17 +92,23 @@ static void handle_db_replace(int fd, if (directory_map == nullptr) { directory_map = new std::map>(); } - directory_map->erase(fd); - directory_map->insert({fd, next_directory_entry}); + auto itr = directory_map->find(fd); + if (itr == directory_map->end()) { + directory_map->insert({fd, next_directory_entry}); + } else { + directory_map->erase(itr); + directory_map->insert({fd, next_directory_entry}); + } LeaveCriticalSection(&g_critical_section.critical_section_); } +const std::size_t kDirectoryInfoBufferSize = + kSbFileMaxPath + sizeof(FILE_ID_BOTH_DIR_INFO); + std::deque GetDirectoryEntries(HANDLE directory_handle) { // According to // https://msdn.microsoft.com/en-us/library/windows/desktop/aa364226(v=vs.85).aspx, // FILE_ID_BOTH_DIR_INFO must be aligned on a DWORDLONG boundary. - const std::size_t kDirectoryInfoBufferSize = - kSbFileMaxPath + sizeof(FILE_ID_BOTH_DIR_INFO); alignas(sizeof(DWORDLONG)) std::vector directory_info_buffer( kDirectoryInfoBufferSize); @@ -146,21 +152,24 @@ DIR* opendir(const char* path) { using starboard::shared::win32::NormalizeWin32Path; if ((path == nullptr) || (path[0] == '\0')) { - return NULL; + errno = ENOENT; + return nullptr; } std::wstring path_wstring = NormalizeWin32Path(path); if (!starboard::shared::win32::IsAbsolutePath(path_wstring)) { - return NULL; + errno = EBADF; + return nullptr; } - SbFileError* out_error; + SbFileError out_error; HANDLE directory_handle = starboard::shared::win32::OpenFileOrDirectory( - path, kSbFileOpenOnly | kSbFileRead, nullptr, out_error); + path, kSbFileOpenOnly | kSbFileRead, nullptr, &out_error); if (!starboard::shared::win32::IsValidHandle(directory_handle)) { - return NULL; + errno = EBADF; + return nullptr; } FILE_BASIC_INFO basic_info = {0}; @@ -170,20 +179,29 @@ DIR* opendir(const char* path) { if (!basic_info_success || !(basic_info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { CloseHandle(directory_handle); - return NULL; + errno = ENOTDIR; + return nullptr; } - DIR* dir = reinterpret_cast(malloc(sizeof(DIR))); + DIR* dir = reinterpret_cast(calloc(1, sizeof(DIR))); dir->handle = directory_handle; dir->fd = handle_db_put(std::deque()); return dir; } int closedir(DIR* dir) { + if (!dir) { + errno = EBADF; + return -1; + } bool success = CloseHandle(dir->handle); handle_db_get(dir->fd, true); - delete dir; - return success ? 0 : -1; + free(dir); + if (!success) { + errno = EBADF; + return -1; + } + return 0; } int readdir_r(DIR* __restrict dir, @@ -191,7 +209,7 @@ int readdir_r(DIR* __restrict dir, struct dirent** __restrict dirent) { if (!dir || !dirent_buf || !dirent) { *dirent = NULL; - return -1; + return EBADF; } auto next_directory_entries = handle_db_get(dir->fd, false); @@ -201,14 +219,14 @@ int readdir_r(DIR* __restrict dir, if (next_directory_entries.empty()) { *dirent = NULL; - return -1; + return ENOENT; } if (starboard::strlcpy(dirent_buf->d_name, next_directory_entries.rbegin()->c_str(), kSbFileMaxName) >= kSbFileMaxName) { *dirent = NULL; - return -1; + return ENOENT; } *dirent = dirent_buf; next_directory_entries.pop_back();