From b8b8fa35fb20f8c4d65e2963f806903e8014b922 Mon Sep 17 00:00:00 2001 From: cobalt-github-releaser-bot <95661244+cobalt-github-releaser-bot@users.noreply.github.com> Date: Sun, 23 Jun 2024 20:21:15 -0700 Subject: [PATCH] Cherry pick PR #3243: Clean up SbFile usages (#3652) Refer to the original PR: https://github.com/youtube/cobalt/pull/3243 b/302715109 Co-authored-by: Hao <131711973+haozheng-cobalt@users.noreply.github.com> --- base/files/file.h | 11 +- base/files/file_enumerator.h | 14 +- base/files/file_enumerator_starboard.cc | 19 +- base/files/file_enumerator_unittest.cc | 33 +- base/files/file_starboard.cc | 291 +++++++++++------- base/files/file_unittest.cc | 47 ++- base/files/file_util_starboard.cc | 160 ++++------ base/files/file_util_unittest.cc | 25 +- base/files/platform_file.h | 6 +- base/files/scoped_file.cc | 2 + base/files/scoped_file.h | 20 +- base/logging.cc | 28 +- cobalt/base/tokens.h | 3 + cobalt/media/sandbox/format_guesstimator.cc | 3 +- .../media/sandbox/web_media_player_sandbox.cc | 11 +- .../skia/skia/src/ports/SkOSFile_cobalt.cc | 4 +- cobalt/renderer/test/png_utils/png_decode.cc | 13 +- cobalt/renderer/test/png_utils/png_encode.cc | 2 +- cobalt/speech/microphone_fake.cc | 3 +- cobalt/trace_event/json_file_outputter.cc | 3 + cobalt/watchdog/watchdog.cc | 2 +- cobalt/websocket/web_socket.h | 3 + .../proxy_config_service_linux.cc | 3 +- starboard/common/file.h | 61 ++-- starboard/loader_app/app_key_files.cc | 11 +- starboard/loader_app/drain_file_helper.cc | 4 +- starboard/loader_app/drain_file_test.cc | 4 +- .../loader_app/installation_manager_test.cc | 5 +- .../loader_app/reset_evergreen_update_test.cc | 12 +- .../storage_test.cc | 10 +- .../posix_socket_send_test.cc | 2 +- .../posix_socket_sendto_test.cc | 2 +- starboard/nplb/system_get_path_test.cc | 7 +- .../linux/system_get_used_cpu_memory.cc | 3 +- starboard/shared/starboard/link_receiver.cc | 24 +- .../shared/starboard/localized_strings.cc | 17 +- .../starboard/player/file_cache_reader.cc | 3 +- .../filter/testing/file_cache_reader_test.cc | 3 +- starboard/shared/uwp/log_writer_win32.cc | 9 +- starboard/shared/widevine/widevine_storage.cc | 4 +- .../shared/win32/posix_emu/include/fcntl.h | 6 +- .../shared/win32/posix_emu/include/unistd.h | 11 +- starboard/shared/win32/posix_emu/socket.cc | 10 +- .../api_leak_detector/api_leak_detector.py | 1 + starboard/win/win32/test_filters.py | 4 + .../googletest/src/googletest/src/gtest.cc | 7 +- third_party/musl/src/starboard/sys/stat.c | 5 + 47 files changed, 520 insertions(+), 411 deletions(-) diff --git a/base/files/file.h b/base/files/file.h index 21528069f485..855f763e8d9e 100644 --- a/base/files/file.h +++ b/base/files/file.h @@ -22,7 +22,11 @@ struct stat; namespace base { +#if defined(STARBOARD) +using stat_wrapper_t = struct ::stat; +#else using stat_wrapper_t = struct stat; +#endif // Thin wrapper around an OS-level file. // Note that this class does not provide any support for asynchronous IO, other @@ -118,7 +122,7 @@ class BASE_EXPORT File { struct BASE_EXPORT Info { Info(); ~Info(); -#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) +#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) || defined(STARBOARD) // Fills this struct with values from |stat_info|. void FromStat(const stat_wrapper_t& stat_info); #endif @@ -370,11 +374,14 @@ class BASE_EXPORT File { // Converts an error value to a human-readable form. Used for logging. static std::string ErrorToString(Error error); -#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) +#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) || defined(STARBOARD) // Wrapper for stat() or stat64(). static int Stat(const char* path, stat_wrapper_t* sb); static int Fstat(int fd, stat_wrapper_t* sb); +# if !defined(STARBOARD) + // Starboard does not support lstat yet. static int Lstat(const char* path, stat_wrapper_t* sb); +#endif #endif // This function can be used to augment `flags` with the correct flags diff --git a/base/files/file_enumerator.h b/base/files/file_enumerator.h index 85bd451654a1..b970055bc8aa 100644 --- a/base/files/file_enumerator.h +++ b/base/files/file_enumerator.h @@ -18,6 +18,8 @@ #include "build/build_config.h" #if defined(STARBOARD) +#include +#include #include "starboard/file.h" #elif BUILDFLAG(IS_WIN) #include "base/win/windows_types.h" @@ -60,7 +62,8 @@ class BASE_EXPORT FileEnumerator { // On POSIX systems, this is rounded down to the second. Time GetLastModifiedTime() const; -#if defined(STARBOARD) +#if defined(STARBOARD) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) + const stat_wrapper_t& stat() const { return stat_; } #elif BUILDFLAG(IS_WIN) // Note that the cAlternateFileName (used to hold the "short" 8.3 name) // of the WIN32_FIND_DATA will be empty. Since we don't use short file @@ -68,21 +71,16 @@ class BASE_EXPORT FileEnumerator { const WIN32_FIND_DATA& find_data() const { return *ChromeToWindowsType(&find_data_); } -#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) - const stat_wrapper_t& stat() const { return stat_; } #endif private: friend class FileEnumerator; -#if defined(STARBOARD) +#if defined(STARBOARD) || BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) + stat_wrapper_t stat_; FilePath filename_; - SbFileInfo sb_info_; #elif BUILDFLAG(IS_WIN) CHROME_WIN32_FIND_DATA find_data_; -#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) - stat_wrapper_t stat_; - FilePath filename_; #endif }; diff --git a/base/files/file_enumerator_starboard.cc b/base/files/file_enumerator_starboard.cc index 9b0c3472943d..5ec4f92793bc 100644 --- a/base/files/file_enumerator_starboard.cc +++ b/base/files/file_enumerator_starboard.cc @@ -28,11 +28,11 @@ namespace base { // FileEnumerator::FileInfo ---------------------------------------------------- FileEnumerator::FileInfo::FileInfo() { - memset(&sb_info_, 0, sizeof(sb_info_)); + memset(&stat_, 0, sizeof(stat_)); } bool FileEnumerator::FileInfo::IsDirectory() const { - return sb_info_.is_directory; + return S_ISDIR(stat_.st_mode); } FilePath FileEnumerator::FileInfo::GetName() const { @@ -40,12 +40,11 @@ FilePath FileEnumerator::FileInfo::GetName() const { } int64_t FileEnumerator::FileInfo::GetSize() const { - return sb_info_.size; + return stat_.st_size; } base::Time FileEnumerator::FileInfo::GetLastModifiedTime() const { - return base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(sb_info_.last_modified)); + return base::Time::FromTimeT(stat_.st_mtime); } // FileEnumerator -------------------------------------------------------------- @@ -124,9 +123,9 @@ std::vector FileEnumerator::ReadDirectory( FilePath full_name = source.Append(filename); // TODO: Make sure this follows symlinks on relevant platforms. - if (!SbFileGetPathInfo(full_name.value().c_str(), &info.sb_info_)) { + if (stat(full_name.value().c_str(), &info.stat_) != 0) { DPLOG(ERROR) << "Couldn't SbFileGetInfo on " << full_name.value(); - memset(&info.sb_info_, 0, sizeof(info.sb_info_)); + memset(&info.stat_, 0, sizeof(info.stat_)); } return info; }; @@ -186,12 +185,12 @@ FilePath FileEnumerator::Next() { continue; } - if (recursive_ && file_info.sb_info_.is_directory) { + if (recursive_ && file_info.IsDirectory()) { pending_paths_.push(full_path); } - if ((file_info.sb_info_.is_directory && (file_type_ & DIRECTORIES)) || - (!file_info.sb_info_.is_directory && (file_type_ & FILES)) || + if ((file_info.IsDirectory() && (file_type_ & DIRECTORIES)) || + (!file_info.IsDirectory() && (file_type_ & FILES)) || (file_type_ & NAMES_ONLY)) { directory_entries_.push_back(file_info); } diff --git a/base/files/file_enumerator_unittest.cc b/base/files/file_enumerator_unittest.cc index 05407f36acce..8ef751ad7692 100644 --- a/base/files/file_enumerator_unittest.cc +++ b/base/files/file_enumerator_unittest.cc @@ -103,7 +103,7 @@ bool CreateDummyFile(const FilePath& path) { bool GetFileInfo(const FilePath& file_path, File::Info& info) { // FLAG_WIN_BACKUP_SEMANTICS: Needed to open directories on Windows. File f(file_path, - File::FLAG_OPEN | File::FLAG_READ | File::FLAG_WIN_BACKUP_SEMANTICS); + File::FLAG_OPEN | File::FLAG_READ | File::FLAG_WIN_BACKUP_SEMANTICS); if (!f.IsValid()) { LOG(ERROR) << "Could not open " << file_path.value() << ": " << File::ErrorToString(f.error_details()); @@ -520,7 +520,15 @@ TEST(FileEnumerator, GetInfoRecursive) { // all the files. for (TestDirectory& dir : directories) { const FilePath dir_path = temp_dir.GetPath().Append(dir.name); +#if defined(STARBOARD) +#ifdef _WIN32 +// TODO: Reable this test when support directory open in base::File for Windows. +// Below tests would fail because we are now using _open from instead of +// CreateFile from . +#else ASSERT_TRUE(GetFileInfo(dir_path, dir.info)); +#endif +#endif } FileEnumerator file_enumerator( @@ -530,6 +538,10 @@ TEST(FileEnumerator, GetInfoRecursive) { auto info = file_enumerator.GetInfo(); bool found = false; if (info.IsDirectory()) { +#if defined(STARBOARD) +#ifdef _WIN32 +// Reable this test when support directory open in base::File for Windows. +#else for (TestDirectory& dir : directories) { if (info.GetName() == dir.name) { CheckDirectoryAgainstInfo(info, dir); @@ -537,6 +549,8 @@ TEST(FileEnumerator, GetInfoRecursive) { break; } } +#endif +#endif } else { for (TestFile& file : files) { if (info.GetName() == file.path.BaseName()) { @@ -546,14 +560,25 @@ TEST(FileEnumerator, GetInfoRecursive) { } } } - +#if defined(STARBOARD) +#ifdef _WIN32 +// Reable this test when support directory open in base::File for Windows. +#else EXPECT_TRUE(found) << "Got unexpected result " << info.GetName().value(); +#endif +#endif } + #if defined(STARBOARD) +#ifdef _WIN32 +// Reable this test when support directory open in base::File for Windows. +#else for (const TestDirectory& dir : directories) { EXPECT_TRUE(dir.found) << "Directory " << dir.name.value() << " was not returned"; } +#endif +#endif for (const TestFile& file : files) { EXPECT_TRUE(file.found) << "File " << file.path.value() << " was not returned"; @@ -569,6 +594,10 @@ TEST(FileEnumerator, GetInfoRecursive) { // a bug in Windows, not us -- you can see it with the "dir" command (notice // that the time of . and .. always match). Skip this test. // https://crbug.com/1119546 +#elif defined(STARBOARD) +#ifdef _WIN32 +// Reable this test when support directory open in base::File for Windows. +#endif #else // Tests that FileEnumerator::GetInfo() returns the correct info for the .. // directory. diff --git a/base/files/file_starboard.cc b/base/files/file_starboard.cc index 86d5f21b2971..53ff49dedbda 100644 --- a/base/files/file_starboard.cc +++ b/base/files/file_starboard.cc @@ -13,14 +13,18 @@ // limitations under the License. // Adapted from platform_file_posix.cc - +#include "base/logging.h" #include "base/files/file_starboard.h" #include +#include +#include +#include #include "base/files/file.h" #include "base/files/file_path.h" #include "base/notreached.h" +#include "base/posix/eintr_wrapper.h" #include "base/strings/stringprintf.h" #include "base/threading/thread_restrictions.h" #include "starboard/common/metrics/stats_tracker.h" @@ -32,6 +36,7 @@ namespace { SbFileError g_sb_file_error = kSbFileOk; } // namespace +// TODO: remove SbFileError void SetLastFileError(File::Error error) { g_sb_file_error = static_cast(error); } @@ -53,6 +58,68 @@ static_assert(File::FROM_BEGIN == static_cast(kSbFileFromBegin) && File::FROM_END == static_cast(kSbFileFromEnd), "Whence enums from base must match those of Starboard."); +void File::Info::FromStat(const stat_wrapper_t& stat_info) { + is_directory = S_ISDIR(stat_info.st_mode); + is_symbolic_link = S_ISLNK(stat_info.st_mode); + size = stat_info.st_size; + + // Get last modification time, last access time, and creation time from + // |stat_info|. + // Note: st_ctime is actually last status change time when the inode was last + // updated, which happens on any metadata change. It is not the file's + // creation time. However, other than on Mac & iOS where the actual file + // creation time is included as st_birthtime, the rest of POSIX platforms have + // no portable way to get the creation time. +#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_FUCHSIA) + time_t last_modified_sec = stat_info.st_mtim.tv_sec; + int64_t last_modified_nsec = stat_info.st_mtim.tv_nsec; + time_t last_accessed_sec = stat_info.st_atim.tv_sec; + int64_t last_accessed_nsec = stat_info.st_atim.tv_nsec; + time_t creation_time_sec = stat_info.st_ctim.tv_sec; + int64_t creation_time_nsec = stat_info.st_ctim.tv_nsec; +#elif BUILDFLAG(IS_ANDROID) + time_t last_modified_sec = stat_info.st_mtime; + int64_t last_modified_nsec = stat_info.st_mtime_nsec; + time_t last_accessed_sec = stat_info.st_atime; + int64_t last_accessed_nsec = stat_info.st_atime_nsec; + time_t creation_time_sec = stat_info.st_ctime; + int64_t creation_time_nsec = stat_info.st_ctime_nsec; +#elif BUILDFLAG(IS_APPLE) + time_t last_modified_sec = stat_info.st_mtimespec.tv_sec; + int64_t last_modified_nsec = stat_info.st_mtimespec.tv_nsec; + time_t last_accessed_sec = stat_info.st_atimespec.tv_sec; + int64_t last_accessed_nsec = stat_info.st_atimespec.tv_nsec; + time_t creation_time_sec = stat_info.st_birthtimespec.tv_sec; + int64_t creation_time_nsec = stat_info.st_birthtimespec.tv_nsec; +#elif BUILDFLAG(IS_BSD) + time_t last_modified_sec = stat_info.st_mtimespec.tv_sec; + int64_t last_modified_nsec = stat_info.st_mtimespec.tv_nsec; + time_t last_accessed_sec = stat_info.st_atimespec.tv_sec; + int64_t last_accessed_nsec = stat_info.st_atimespec.tv_nsec; + time_t creation_time_sec = stat_info.st_ctimespec.tv_sec; + int64_t creation_time_nsec = stat_info.st_ctimespec.tv_nsec; +#else + time_t last_modified_sec = stat_info.st_mtime; + int64_t last_modified_nsec = 0; + time_t last_accessed_sec = stat_info.st_atime; + int64_t last_accessed_nsec = 0; + time_t creation_time_sec = stat_info.st_ctime; + int64_t creation_time_nsec = 0; +#endif + + last_modified = + Time::FromTimeT(last_modified_sec) + + Microseconds(last_modified_nsec / Time::kNanosecondsPerMicrosecond); + + last_accessed = + Time::FromTimeT(last_accessed_sec) + + Microseconds(last_accessed_nsec / Time::kNanosecondsPerMicrosecond); + + creation_time = + Time::FromTimeT(creation_time_sec) + + Microseconds(creation_time_nsec / Time::kNanosecondsPerMicrosecond); +} + bool File::IsValid() const { return file_.is_valid(); } @@ -79,7 +146,8 @@ int64_t File::Seek(Whence whence, int64_t offset) { DCHECK(IsValid()); SCOPED_FILE_TRACE_WITH_SIZE("Seek", offset); - return SbFileSeek(file_.get(), static_cast(whence), offset); + return lseek(file_.get(), static_cast(offset), + static_cast(whence)); } int File::Read(int64_t offset, char* data, int size) { @@ -91,19 +159,21 @@ int File::Read(int64_t offset, char* data, int size) { SCOPED_FILE_TRACE_WITH_SIZE("Read", size); - int original_position = SbFileSeek(file_.get(), kSbFileFromCurrent, 0); + int original_position = lseek(file_.get(), 0, static_cast(SEEK_CUR)); if (original_position < 0) { return -1; } - int position = SbFileSeek(file_.get(), kSbFileFromBegin, offset); + int position = + lseek(file_.get(), static_cast(offset), static_cast(SEEK_SET)); int result = 0; if (position == offset) { result = ReadAtCurrentPos(data, size); } // Restore position regardless of result of write. - position = SbFileSeek(file_.get(), kSbFileFromBegin, original_position); + position = + lseek(file_.get(), static_cast(original_position), static_cast(SEEK_SET)); if (result < 0) { return result; } @@ -123,7 +193,7 @@ int File::ReadAtCurrentPos(char* data, int size) { SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPos", size); - return SbFileReadAll(file_.get(), data, size); + return starboard::ReadAll(file_.get(), data, size); } int File::ReadNoBestEffort(int64_t offset, char* data, int size) { @@ -131,19 +201,22 @@ int File::ReadNoBestEffort(int64_t offset, char* data, int size) { DCHECK(IsValid()); SCOPED_FILE_TRACE_WITH_SIZE("ReadNoBestEffort", size); - int original_position = SbFileSeek(file_.get(), kSbFileFromCurrent, 0); + int original_position = lseek( + file_.get(), 0, static_cast(SEEK_CUR)); if (original_position < 0) { return -1; } - int position = SbFileSeek(file_.get(), kSbFileFromBegin, offset); + int position = + lseek(file_.get(), static_cast(offset), static_cast(SEEK_SET)); int result = 0; if (position == offset) { - result = SbFileRead(file_.get(), data, size); + result = read(file_.get(), data, size); } // Restore position regardless of result of read. - position = SbFileSeek(file_.get(), kSbFileFromBegin, original_position); + position = lseek(file_.get(), static_cast(original_position), + static_cast(SEEK_SET)); if (result < 0) { return result; } @@ -162,29 +235,31 @@ int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { return -1; SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPosNoBestEffort", size); - return SbFileRead(file_.get(), data, size); + return read(file_.get(), data, size); } int File::Write(int64_t offset, const char* data, int size) { internal::AssertBlockingAllowed(); - if (append_) { return WriteAtCurrentPos(data, size); } - int original_position = SbFileSeek(file_.get(), kSbFileFromCurrent, 0); + int original_position = lseek(file_.get(), 0, static_cast(SEEK_CUR)); if (original_position < 0) { return -1; } - int64_t position = SbFileSeek(file_.get(), kSbFileFromBegin, offset); + int64_t position = + lseek(file_.get(), static_cast(offset), static_cast(SEEK_SET)); + int result = 0; if (position == offset) { result = WriteAtCurrentPos(data, size); } // Restore position regardless of result of write. - position = SbFileSeek(file_.get(), kSbFileFromBegin, original_position); + position = lseek(file_.get(), static_cast(original_position), + static_cast(SEEK_SET)); if (result < 0) { return result; } @@ -203,9 +278,19 @@ int File::WriteAtCurrentPos(const char* data, int size) { return -1; SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPos", size); - int write_result = SbFileWriteAll(file_.get(), data, size); - RecordFileWriteStat(write_result); - return write_result; + + int bytes_written = 0; + long rv; + do { + rv = HANDLE_EINTR(write(file_.get(), data + bytes_written, + static_cast(size - bytes_written))); + if (rv <= 0) + break; + + bytes_written += rv; + } while (bytes_written < size); + + return bytes_written ? bytes_written : checked_cast(rv); } int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { @@ -215,7 +300,7 @@ int File::WriteAtCurrentPosNoBestEffort(const char* data, int size) { return -1; SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPosNoBestEffort", size); - int write_result = SbFileWrite(file_.get(), data, size); + int write_result = write(file_.get(), data, size); RecordFileWriteStat(write_result); return write_result; } @@ -225,20 +310,18 @@ int64_t File::GetLength() { SCOPED_FILE_TRACE("GetLength"); - File::Info file_info; - if (!GetInfo(&file_info)) { + stat_wrapper_t file_info; + if (Fstat(file_.get(), &file_info)) return -1; - } - return file_info.size; + return file_info.st_size; } bool File::SetLength(int64_t length) { internal::AssertBlockingAllowed(); DCHECK(IsValid()); - SCOPED_FILE_TRACE_WITH_SIZE("SetLength", length); - return SbFileTruncate(file_.get(), length); + return !ftruncate(file_.get(), length); } bool File::SetTimes(Time last_access_time, Time last_modified_time) { @@ -256,71 +339,61 @@ bool File::GetInfo(Info* info) { SCOPED_FILE_TRACE("GetInfo"); - if (!info || !SbFileIsValid(file_.get())) - return false; - - SbFileInfo file_info; - if (!SbFileGetInfo(file_.get(), &file_info)) + stat_wrapper_t file_info; + if (Fstat(file_.get(), &file_info)) return false; - info->is_directory = file_info.is_directory; - info->is_symbolic_link = file_info.is_symbolic_link; - info->size = file_info.size; - info->last_modified = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(file_info.last_modified)); - info->last_accessed = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(file_info.last_accessed)); - info->creation_time = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(file_info.creation_time)); + info->FromStat(file_info); return true; } File::Error File::GetLastFileError() { - return base::File::OSErrorToFileError(g_sb_file_error); + return base::File::OSErrorToFileError(errno); } // Static. -File::Error File::OSErrorToFileError(SbSystemError sb_system_error) { - switch (static_cast(sb_system_error)) { - case kSbFileOk: - return FILE_OK; - case kSbFileErrorFailed: - return FILE_ERROR_FAILED; - case kSbFileErrorInUse: +int File::Stat(const char* path, stat_wrapper_t* sb) { + internal::AssertBlockingAllowed(); + return stat(path, sb); +} +int File::Fstat(int fd, stat_wrapper_t* sb) { + internal::AssertBlockingAllowed(); + return fstat(fd, sb); +} + +// Static. +File::Error File::OSErrorToFileError(int saved_errno) { + switch (saved_errno) { + case EACCES: + case EISDIR: + case EROFS: + case EPERM: + return FILE_ERROR_ACCESS_DENIED; + case EBUSY: +#if !BUILDFLAG(IS_NACL) // ETXTBSY not defined by NaCl. + case ETXTBSY: +#endif return FILE_ERROR_IN_USE; - case kSbFileErrorExists: + case EEXIST: return FILE_ERROR_EXISTS; - case kSbFileErrorNotFound: + case EIO: + return FILE_ERROR_IO; + case ENOENT: return FILE_ERROR_NOT_FOUND; - case kSbFileErrorAccessDenied: - return FILE_ERROR_ACCESS_DENIED; - case kSbFileErrorTooManyOpened: + case ENFILE: // fallthrough + case EMFILE: return FILE_ERROR_TOO_MANY_OPENED; - case kSbFileErrorNoMemory: + case ENOMEM: return FILE_ERROR_NO_MEMORY; - case kSbFileErrorNoSpace: + case ENOSPC: return FILE_ERROR_NO_SPACE; - case kSbFileErrorNotADirectory: + case ENOTDIR: return FILE_ERROR_NOT_A_DIRECTORY; - case kSbFileErrorInvalidOperation: - return FILE_ERROR_INVALID_OPERATION; - case kSbFileErrorSecurity: - return FILE_ERROR_SECURITY; - case kSbFileErrorAbort: - return FILE_ERROR_ABORT; - case kSbFileErrorNotAFile: - return FILE_ERROR_NOT_A_FILE; - case kSbFileErrorNotEmpty: - return FILE_ERROR_NOT_EMPTY; - case kSbFileErrorInvalidUrl: - return FILE_ERROR_INVALID_URL; - case kSbFileErrorIO: - return FILE_ERROR_IO; default: - NOTREACHED() << "Unrecognized SbSystemError: " << sb_system_error; - break; + // This function should only be called for errors. + DCHECK_NE(0, saved_errno); + return FILE_ERROR_FAILED; } - return FILE_ERROR_FAILED; } void File::DoInitialize(const FilePath& path, uint32_t flags) { @@ -332,51 +405,61 @@ void File::DoInitialize(const FilePath& path, uint32_t flags) { file_name_ = path.AsUTF8Unsafe(); int open_flags = 0; - switch (flags & (FLAG_OPEN | FLAG_CREATE | FLAG_OPEN_ALWAYS | - FLAG_CREATE_ALWAYS | FLAG_OPEN_TRUNCATED)) { - case FLAG_OPEN: - open_flags = kSbFileOpenOnly; - break; - - case FLAG_CREATE: - open_flags = kSbFileCreateOnly; - break; - - case FLAG_OPEN_ALWAYS: - open_flags = kSbFileOpenAlways; - break; - - case FLAG_CREATE_ALWAYS: - open_flags = kSbFileCreateAlways; - break; + if (flags & FLAG_CREATE) { + open_flags = O_CREAT | O_EXCL; + } - case FLAG_OPEN_TRUNCATED: - open_flags = kSbFileOpenTruncated; - DCHECK(flags & FLAG_WRITE); - break; + if (flags & FLAG_CREATE_ALWAYS) { + SB_DCHECK(!open_flags); + open_flags = O_CREAT | O_TRUNC; + } - default: - NOTREACHED() << "Passed incompatible flags: " << flags; - error_details_ = FILE_ERROR_FAILED; + if (flags & FLAG_OPEN_TRUNCATED) { + SB_DCHECK(!open_flags); + SB_DCHECK(flags & FLAG_WRITE); + open_flags = O_TRUNC; } - if (flags & FLAG_READ) { - open_flags |= kSbFileRead; + if (!open_flags && !(flags & FLAG_OPEN) && !(flags & FLAG_OPEN_ALWAYS)) { + SB_NOTREACHED(); + errno = EOPNOTSUPP; } if (flags & FLAG_WRITE || flags & FLAG_APPEND) { - open_flags |= kSbFileWrite; + if (flags & FLAG_READ) { + open_flags |= O_RDWR; + } else { + open_flags |= O_WRONLY; + } } - file_.reset(SbFileOpen(path.value().c_str(), open_flags, &created_, - &g_sb_file_error)); + SB_COMPILE_ASSERT(O_RDONLY == 0, O_RDONLY_must_equal_zero); + + int mode = S_IRUSR | S_IWUSR; + int descriptor = HANDLE_EINTR(open(path.value().c_str(), open_flags, mode)); + + if (flags & FLAG_OPEN_ALWAYS) { + if (descriptor < 0) { + open_flags |= O_CREAT; + descriptor = HANDLE_EINTR(open(path.value().c_str(), open_flags, mode)); + if (descriptor >= 0) + created_ = true; + } + } + + if (descriptor >= 0 && + (flags & (FLAG_CREATE_ALWAYS | FLAG_CREATE))) { + created_ = true; + } + + file_.reset(descriptor); if (!file_.is_valid()) { - error_details_ = OSErrorToFileError(g_sb_file_error); + error_details_ = File::GetLastFileError(); } else { error_details_ = FILE_OK; if (append_) { - SbFileSeek(file_.get(), kSbFileFromEnd, 0); + lseek(file_.get(), 0, SEEK_END); } } @@ -392,7 +475,7 @@ bool File::Flush() { DCHECK(IsValid()); SCOPED_FILE_TRACE("Flush"); - return SbFileFlush(file_.get()); + return !fsync(file_.get()); } void File::SetPlatformFile(PlatformFile file) { diff --git a/base/files/file_unittest.cc b/base/files/file_unittest.cc index e81663c65984..e8d88d20ce51 100644 --- a/base/files/file_unittest.cc +++ b/base/files/file_unittest.cc @@ -269,9 +269,7 @@ TEST(FileTest, ReadWrite) { } TEST(FileTest, GetLastFileError) { -#if defined(STARBOARD) - SetLastFileError(File::Error::FILE_ERROR_ACCESS_DENIED); -#elif BUILDFLAG(IS_WIN) +#if BUILDFLAG(IS_WIN) ::SetLastError(ERROR_ACCESS_DENIED); #else errno = EACCES; @@ -389,11 +387,25 @@ TEST(FileTest, Length) { #if !BUILDFLAG(IS_FUCHSIA) // Fuchsia doesn't seem to support big files. // Expand the file past the 4 GB limit. - const int64_t kBigFileLength = 5'000'000'000; - EXPECT_TRUE(file.SetLength(kBigFileLength)); - EXPECT_EQ(kBigFileLength, file.GetLength()); - EXPECT_TRUE(GetFileSize(file_path, &file_size)); - EXPECT_EQ(kBigFileLength, file_size); +#if defined(STARBOARD) +#if SB_IS(32_BIT) +// TODO: After POSIX migration WIN32 uses _chsize in ftruncate, and it +// does not support big file: +// https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/chsize?view=msvc-170 +// Before POSXI migration WIN32 was implemented with SetFilePointerEx : +// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfilepointerex +#else + // TODO: Checking why SB_IS(32_BIT) is not set for WIN32, and we have to dynamically check + // sizeof(long) to filter out WIN32. + if (sizeof(long) == 8) { + const int64_t kBigFileLength = 5'000'000'000; + EXPECT_TRUE(file.SetLength(kBigFileLength)); + EXPECT_EQ(kBigFileLength, file.GetLength()); + EXPECT_TRUE(GetFileSize(file_path, &file_size)); + EXPECT_EQ(kBigFileLength, file_size); + } +#endif +#endif #endif // Close the file and reopen with base::File::FLAG_CREATE_ALWAYS, and make @@ -644,15 +656,18 @@ TEST(FileTest, MAYBE_WriteDataToLargeOffset) { const char kData[] = "this file is sparse."; const int kDataLen = sizeof(kData) - 1; + int64_t kLargeFileOffset; + +// TODO: Checking why SB_IS(32_BIT) is not set for WIN32, and we have to dynamically check +// sizeof(long) to filter out WIN32. #if defined(STARBOARD) -#if SB_IS(32_BIT) - // Maximum off_t for lseek() on 32-bit builds is just below 2^31. - const int64_t kLargeFileOffset = (1LL << 31) - 2; -#else // SB_IS(32_BIT) - const int64_t kLargeFileOffset = (1LL << 31); -#endif // SB_IS(32_BIT) -#else // defined(STARBOARD) - const int64_t kLargeFileOffset = (1LL << 31); + if (sizeof(long) == 4) { + kLargeFileOffset = (1LL << 31) - 2; + } else { + kLargeFileOffset = (1LL << 31); +} +#else // defined(STARBOARD) +kLargeFileOffset = (1LL << 31); #endif // defined(STARBOARD) // If the file fails to write, it is probably we are running out of disk space diff --git a/base/files/file_util_starboard.cc b/base/files/file_util_starboard.cc index f10ac5a98310..f29d0cdc3b92 100644 --- a/base/files/file_util_starboard.cc +++ b/base/files/file_util_starboard.cc @@ -16,7 +16,10 @@ #include "base/files/file_util.h" +#include +#include #include +#include #include #include @@ -78,24 +81,24 @@ void GenerateTempFileName(FilePath::StringType *in_out_template) { // Creates a random filename based on the TempFileName() pattern, creates the // file, leaving it open, placing the path in |out_path| and returning the open -// SbFile. -SbFile CreateAndOpenTemporaryFile(FilePath directory, FilePath *out_path) { +// file. +int CreateAndOpenTemporaryFile(FilePath directory, FilePath *out_path) { internal::AssertBlockingAllowed(); DCHECK(out_path); FilePath path = directory.Append(TempFileName()); FilePath::StringType tmpdir_string = path.value(); GenerateTempFileName(&tmpdir_string); *out_path = FilePath(tmpdir_string); - return SbFileOpen(tmpdir_string.c_str(), kSbFileCreateOnly | kSbFileWrite, - NULL, NULL); + return open(tmpdir_string.c_str(), O_CREAT | O_EXCL | O_WRONLY, + S_IRUSR | S_IWUSR); } // Retries creating a temporary file until it can win the race to create a // unique one. -SbFile CreateAndOpenTemporaryFileSafely(FilePath directory, +int CreateAndOpenTemporaryFileSafely(FilePath directory, FilePath *out_path) { - SbFile file = kSbFileInvalid; - while (!SbFileIsValid(file)) { + int file = -1; + while (file < 0) { file = CreateAndOpenTemporaryFile(directory, out_path); } return file; @@ -145,58 +148,53 @@ bool AbsolutePath(FilePath* path) { return path->IsAbsolute(); } -bool DeleteFile(const FilePath &path, bool recursive) { - internal::AssertBlockingAllowed(); - const char *path_str = path.value().c_str(); - struct ::stat info; - bool directory = ::stat(path_str, &info) == 0 && S_ISDIR(info.st_mode); - if (!recursive || !directory) { - return SbFileDelete(path_str); +// Borrowed from file_util_posix.cc +bool DoDeleteFile(const FilePath& path, bool recursive) { + ScopedBlockingCall scoped_blocking_call(FROM_HERE, BlockingType::MAY_BLOCK); + // Reset errno in the beginning of the funciton + errno = 0; + + const char* path_str = path.value().c_str(); + stat_wrapper_t file_info; + if (::stat(path_str, &file_info) != 0) { + return (errno == ENOENT || errno == ENOTDIR); + } + if (!S_ISDIR(file_info.st_mode)) { + return (unlink(path_str) == 0) || (errno == ENOENT); + } + if (!recursive){ + return (rmdir(path_str) == 0) || (errno == ENOENT); } bool success = true; - std::stack directories; + stack directories; directories.push(path.value()); - - // NOTE: Right now, for Linux, SbFileGetInfo does not follow - // symlinks. This is good for avoiding deleting through symlinks, but makes it - // hard to use symlinks on platforms where they are supported. This seems - // safest for the lowest-common-denominator approach of pretending symlinks - // don't exist. - FileEnumerator traversal( - path, true, FileEnumerator::FILES | FileEnumerator::DIRECTORIES); - - // Delete all files and push all directories in depth order onto the stack. - for (FilePath current = traversal.Next(); - success && !current.empty(); + FileEnumerator traversal(path, true, + FileEnumerator::FILES | FileEnumerator::DIRECTORIES); + for (FilePath current = traversal.Next(); !current.empty(); current = traversal.Next()) { - FileEnumerator::FileInfo info(traversal.GetInfo()); - - if (info.IsDirectory()) { + if (traversal.GetInfo().IsDirectory()) { directories.push(current.value()); - } else { - success = SbFileDelete(current.value().c_str()); + } + else { + success &= (unlink(current.value().c_str()) == 0) || (errno == ENOENT); } } - // Delete all directories in reverse-depth order, now that they have no more - // regular files. - while (success && !directories.empty()) { - success = SbFileDelete(directories.top().c_str()); + while (!directories.empty()) { + FilePath dir = FilePath(directories.top()); directories.pop(); + success &= (rmdir(dir.value().c_str()) == 0) || (errno == ENOENT); } - return success; } -bool DeletePathRecursively(const FilePath &path) { - bool recursive = true; - return DeleteFile(path, recursive); +bool DeleteFile(const FilePath& path) { + return DoDeleteFile(path, /*recursive=*/false); } -bool DeleteFile(const FilePath &path) { - bool recursive = false; - return DeleteFile(path, recursive); +bool DeletePathRecursively(const FilePath& path) { + return DoDeleteFile(path, /*recursive=*/true); } bool DieFileDie(const FilePath& file, bool recurse) { @@ -219,52 +217,19 @@ bool ReplaceFile(const FilePath& from_path, return true; } -bool CopyFile(const FilePath &from_path, const FilePath &to_path) { - internal::AssertBlockingAllowed(); - - base::File source_file(from_path, base::File::FLAG_OPEN | base::File::FLAG_READ); - if (!source_file.IsValid()) { - DPLOG(ERROR) << "CopyFile(): Unable to open source file: " - << from_path.value(); +// Mac has its own implementation, this is for all other Posix systems. +bool CopyFile(const FilePath& from_path, const FilePath& to_path) { + ScopedBlockingCall scoped_blocking_call(FROM_HERE, BlockingType::MAY_BLOCK); + File infile; + infile = File(from_path, File::FLAG_OPEN | File::FLAG_READ); + if (!infile.IsValid()) return false; - } - base::File destination_file( - to_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); - if (!destination_file.IsValid()) { - DPLOG(ERROR) << "CopyFile(): Unable to open destination file: " - << to_path.value(); + File outfile(to_path, File::FLAG_WRITE | File::FLAG_CREATE_ALWAYS); + if (!outfile.IsValid()) return false; - } - const size_t kBufferSize = 32768; - std::vector buffer(kBufferSize); - bool result = true; - - while (result) { - int bytes_read = source_file.ReadAtCurrentPos(&buffer[0], buffer.size()); - if (bytes_read < 0) { - result = false; - break; - } - - if (bytes_read == 0) { - break; - } - - int bytes_written = - destination_file.WriteAtCurrentPos(&buffer[0], bytes_read); - if (bytes_written < bytes_read) { - DLOG(ERROR) << "CopyFile(): bytes_read (" << bytes_read - << ") > bytes_written (" << bytes_written << ")"; - // Because we use a best-effort write, if we wrote less than what was - // available, something went wrong. - result = false; - break; - } - } - - return result; + return CopyFileContents(infile, outfile); } FILE* OpenFile(const FilePath& filename, const char* mode) { @@ -333,15 +298,15 @@ ScopedFILE CreateAndOpenTemporaryStreamInDir(const FilePath& dir, File CreateAndOpenTemporaryFileInDir(const FilePath &dir, FilePath *temp_file) { internal::AssertBlockingAllowed(); DCHECK(temp_file); - SbFile file = CreateAndOpenTemporaryFileSafely(dir, temp_file); - return SbFileIsValid(file) ? File(std::move(file)) : File(File::GetLastFileError()); + int file = CreateAndOpenTemporaryFileSafely(dir, temp_file); + return file >= 0 ? File(std::move(file)) : File(File::GetLastFileError()); } bool CreateTemporaryFileInDir(const FilePath &dir, FilePath *temp_file) { internal::AssertBlockingAllowed(); DCHECK(temp_file); - SbFile file = CreateAndOpenTemporaryFileSafely(dir, temp_file); - return (SbFileIsValid(file) && SbFileClose(file)); + int file = CreateAndOpenTemporaryFileSafely(dir, temp_file); + return ((file >= 0) && !::close(file)); } bool CreateTemporaryDirInDir(const FilePath &base_dir, @@ -427,20 +392,11 @@ bool IsLink(const FilePath &file_path) { } bool GetFileInfo(const FilePath &file_path, File::Info *results) { - internal::AssertBlockingAllowed(); - SbFileInfo info; - if (!SbFileGetPathInfo(file_path.value().c_str(), &info)) { - return false; - } + stat_wrapper_t file_info; + if (::stat(file_path.value().c_str(), &file_info) != 0) + return false; - results->is_directory = info.is_directory; - results->size = info.size; - results->last_modified = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(info.last_modified)); - results->last_accessed = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(info.last_accessed)); - results->creation_time = base::Time::FromDeltaSinceWindowsEpoch( - base::TimeDelta::FromMicroseconds(info.creation_time)); + results->FromStat(file_info); return true; } diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc index f769fe99e31b..db8776c0ab5c 100644 --- a/base/files/file_util_unittest.cc +++ b/base/files/file_util_unittest.cc @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -46,6 +47,7 @@ #include "base/threading/thread.h" #include "base/time/time.h" #include "build/build_config.h" +#include "starboard/common/file.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/multiprocess_func_list.h" #include "testing/platform_test.h" @@ -336,14 +338,13 @@ void CreateTextFile(const FilePath& filename, const std::wstring& contents) { #if defined(STARBOARD) const std::string contents_ascii = UTF16ToASCII(WideToUTF16(contents)); - SbFileError file_error = kSbFileOk; - SbFile file = - SbFileOpen(filename.value().c_str(), kSbFileCreateAlways | kSbFileWrite, - nullptr, &file_error); - SB_CHECK((file_error == kSbFileOk)); - SB_CHECK(SbFileWriteAll(file, contents_ascii.data(), contents_ascii.size()) == + int file = + open(filename.value().c_str(), O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR); + SB_CHECK(file >= 0); + int ret = starboard::WriteAll(file, contents_ascii.data(),contents_ascii.size()); + SB_CHECK(ret == contents_ascii.size()); - SB_CHECK(SbFileClose(file)); + SB_CHECK(!::close(file)); #else // !defined(STARBOARD) std::wofstream file; #if BUILDFLAG(IS_WIN) @@ -362,12 +363,10 @@ std::wstring ReadTextFile(const FilePath& filename) { #if defined(STARBOARD) const int size_in_bytes = 64 * sizeof(wchar_t); char contents[size_in_bytes]{0}; - SbFileError file_error = kSbFileOk; - SbFile file = SbFileOpen(filename.value().c_str(), - kSbFileOpenOnly | kSbFileRead, nullptr, &file_error); - SB_CHECK(file_error == kSbFileOk); - SB_CHECK(SbFileReadAll(file, contents, size_in_bytes) != -1); - SB_CHECK(SbFileClose(file)); + int file = open(filename.value().c_str(), 0, S_IRUSR | S_IWUSR); + SB_CHECK(file >= 0); + SB_CHECK(starboard::ReadAll(file, contents, size_in_bytes) != -1); + SB_CHECK(!::close(file)); return UTF16ToWide(ASCIIToUTF16(contents)); #else // !defined(STARBOARD) wchar_t contents[64]; diff --git a/base/files/platform_file.h b/base/files/platform_file.h index b7dd1cc69980..c3e9c511ef7e 100644 --- a/base/files/platform_file.h +++ b/base/files/platform_file.h @@ -24,11 +24,11 @@ namespace base { #if defined(STARBOARD) -using PlatformFile = SbFile; +using PlatformFile = int; using ScopedPlatformFile = ::base::ScopedFD; -constexpr PlatformFile kInvalidPlatformFile = kSbFileInvalid; -#elif BUILDFLAG(IS_WIN) +constexpr PlatformFile kInvalidPlatformFile = -1; +#elif BUILDFLAG(IS_WIN) using PlatformFile = HANDLE; using ScopedPlatformFile = ::base::win::ScopedHandle; diff --git a/base/files/scoped_file.cc b/base/files/scoped_file.cc index d48e09ce275a..67148c5e0381 100644 --- a/base/files/scoped_file.cc +++ b/base/files/scoped_file.cc @@ -8,6 +8,8 @@ #include "build/build_config.h" #if defined(STARBOARD) +#include +#include #elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) #include #include diff --git a/base/files/scoped_file.h b/base/files/scoped_file.h index 1fd962b8806b..f0a57f283a44 100644 --- a/base/files/scoped_file.h +++ b/base/files/scoped_file.h @@ -8,6 +8,7 @@ #include #include +#include #include "base/base_export.h" #include "base/scoped_generic.h" @@ -24,9 +25,9 @@ namespace base { namespace internal { #if defined(STARBOARD) -struct BASE_EXPORT ScopedSbFileCloseTraits { - static SbFile InvalidValue() { return kSbFileInvalid; } - static void Free(SbFile file) { SbFileClose(file); } +struct BASE_EXPORT ScopedFDCloseTraits { + static int InvalidValue() { return -1; } + static void Free(int file) { if (file >= 0) {close(file);} } }; #elif BUILDFLAG(IS_ANDROID) // Use fdsan on android. @@ -58,9 +59,12 @@ struct BASE_EXPORT ScopedFDCloseTraits { #if defined(STARBOARD) // Functor for |ScopedFILE| (below). struct ScopedFILECloser { - inline void operator()(SbFile* x) const { - if (x) - SbFileClose(*x); + inline void operator()(int* x) const { + if (x) { + if (*x >= 0) { + close(*x); + } + } } }; #else @@ -107,9 +111,7 @@ void BASE_EXPORT ResetFDOwnership(); // ----------------------------------------------------------------------------- -#if defined(STARBOARD) -using ScopedFD = ScopedGeneric; -#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) +#if BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) || defined(STARBOARD) // A low-level Posix file descriptor closer class. Use this when writing // platform-specific code, especially that does non-file-like things with the // FD (like sockets). diff --git a/base/logging.cc b/base/logging.cc index 7a42524f1ac7..5900a58e9e7f 100644 --- a/base/logging.cc +++ b/base/logging.cc @@ -39,6 +39,8 @@ #endif // defined(LEAK_SANITIZER) && !BUILDFLAG(IS_NACL) #if defined(STARBOARD) +#include + #include "base/files/file_starboard.h" #include "starboard/client_porting/eztime/eztime.h" #include "starboard/common/log.h" @@ -48,7 +50,7 @@ #include "starboard/configuration_constants.h" #include "starboard/file.h" #include "starboard/system.h" -typedef SbFile FileHandle; +typedef int* FileHandle; typedef pthread_mutex_t MutexHandle; #else #if BUILDFLAG(IS_WIN) @@ -318,7 +320,7 @@ uint64_t TickCount() { void DeleteFilePath(const PathString& log_name) { #if defined(STARBOARD) - SbFileDelete(log_name.c_str()); + unlink(log_name.c_str()); #elif BUILDFLAG(IS_WIN) DeleteFile(log_name.c_str()); #elif BUILDFLAG(IS_NACL) @@ -403,12 +405,13 @@ bool InitializeLogFileHandle() { return true; #if defined(STARBOARD) - g_log_file = SbFileOpen(g_log_file_name->c_str(), - kSbFileOpenAlways | kSbFileWrite, NULL, NULL); - if (!SbFileIsValid(g_log_file)) - return false; + int g_log_file_descriptor = + open(g_log_file_name->c_str(), O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR); + g_log_file = &g_log_file_descriptor; + if (g_log_file_descriptor < 0) + return false; - SbFileSeek(g_log_file, kSbFileFromEnd, 0); + lseek(g_log_file_descriptor, 0, SEEK_END); #elif BUILDFLAG(IS_WIN) // The FILE_APPEND_DATA access mask ensures that the file is atomically // appended to across accesses from multiple threads. @@ -457,7 +460,9 @@ bool InitializeLogFileHandle() { void CloseFile(FileHandle log) { #if defined(STARBOARD) - SbFileClose(log); + if (*log >= 0) { + close(*log); + } #elif BUILDFLAG(IS_WIN) CloseHandle(log); #elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA) @@ -1007,11 +1012,12 @@ LogMessage::~LogMessage() { #endif if (InitializeLogFileHandle()) { #if defined(STARBOARD) - SbFileSeek(g_log_file, kSbFileFromEnd, 0); + lseek(*g_log_file, 0, SEEK_END); int written = 0; while (written < str_newline.length()) { - int result = SbFileWrite(g_log_file, &(str_newline.c_str()[written]), - str_newline.length() - written); + int result = + HANDLE_EINTR(write(*g_log_file, &(str_newline.c_str()[written]), + str_newline.length() - written)); base::RecordFileWriteStat(result); if (result < 0) { break; diff --git a/cobalt/base/tokens.h b/cobalt/base/tokens.h index 024bfc670d63..538c5e46465d 100644 --- a/cobalt/base/tokens.h +++ b/cobalt/base/tokens.h @@ -19,6 +19,9 @@ #include "base/memory/singleton.h" #include "cobalt/base/token.h" +#undef close +#undef open + namespace base { // clang-format off diff --git a/cobalt/media/sandbox/format_guesstimator.cc b/cobalt/media/sandbox/format_guesstimator.cc index 36972f4b512b..843bbd915043 100644 --- a/cobalt/media/sandbox/format_guesstimator.cc +++ b/cobalt/media/sandbox/format_guesstimator.cc @@ -95,8 +95,7 @@ std::vector ReadHeader(const base::FilePath& path) { // Size of the input file to be read into memory for checking the validity // of ChunkDemuxer::AppendToParseBuffer() calls. const int64_t kHeaderSize = 256 * 1024; // 256kb - starboard::ScopedFile file(path.value().c_str(), - kSbFileOpenOnly | kSbFileRead); + starboard::ScopedFile file(path.value().c_str(), 0); int64_t bytes_to_read = std::min(kHeaderSize, file.GetSize()); DCHECK_GE(bytes_to_read, 0); std::vector buffer(bytes_to_read); diff --git a/cobalt/media/sandbox/web_media_player_sandbox.cc b/cobalt/media/sandbox/web_media_player_sandbox.cc index 246c2764dc1c..98516f47ae3e 100644 --- a/cobalt/media/sandbox/web_media_player_sandbox.cc +++ b/cobalt/media/sandbox/web_media_player_sandbox.cc @@ -160,8 +160,7 @@ class Application { std::unique_ptr& file = guesstimator.is_audio() ? audio_file_ : video_file_; - file.reset(new ScopedFile(guesstimator.adaptive_path().c_str(), - kSbFileOpenOnly | kSbFileRead)); + file.reset(new ScopedFile(guesstimator.adaptive_path().c_str(), 0)); if (!file->IsValid()) { LOG(ERROR) << "Failed to open file: " << guesstimator.adaptive_path(); @@ -206,10 +205,10 @@ class Application { const FormatGuesstimator& audio_guesstimator, const FormatGuesstimator& video_guesstimator) { is_adaptive_playback_ = true; - audio_file_.reset(new ScopedFile(audio_guesstimator.adaptive_path().c_str(), - kSbFileOpenOnly | kSbFileRead)); - video_file_.reset(new ScopedFile(video_guesstimator.adaptive_path().c_str(), - kSbFileOpenOnly | kSbFileRead)); + audio_file_.reset( + new ScopedFile(audio_guesstimator.adaptive_path().c_str(), 0)); + video_file_.reset( + new ScopedFile(video_guesstimator.adaptive_path().c_str(), 0)); if (!audio_file_->IsValid()) { LOG(ERROR) << "Failed to open audio file: " diff --git a/cobalt/renderer/rasterizer/skia/skia/src/ports/SkOSFile_cobalt.cc b/cobalt/renderer/rasterizer/skia/skia/src/ports/SkOSFile_cobalt.cc index 2e464f11932d..e108d495b346 100644 --- a/cobalt/renderer/rasterizer/skia/skia/src/ports/SkOSFile_cobalt.cc +++ b/cobalt/renderer/rasterizer/skia/skia/src/ports/SkOSFile_cobalt.cc @@ -50,7 +50,9 @@ int ToSbFileFlags(SkFILE_Flags sk_flags) { FILE* sk_fopen(const char path[], SkFILE_Flags sk_flags) { SbFile starboard_file = SbFileOpen(path, ToSbFileFlags(sk_flags), NULL, NULL); - if (starboard_file == base::kInvalidPlatformFile) { + // TODO: temporarily replace with kSbFileInvalid, will be deprecated with + // SBFile. + if (starboard_file == kSbFileInvalid) { return nullptr; } diff --git a/cobalt/renderer/test/png_utils/png_decode.cc b/cobalt/renderer/test/png_utils/png_decode.cc index 544e83dbf436..a664dd6eda86 100644 --- a/cobalt/renderer/test/png_utils/png_decode.cc +++ b/cobalt/renderer/test/png_utils/png_decode.cc @@ -14,6 +14,9 @@ #include "cobalt/renderer/test/png_utils/png_decode.h" +#include +#include + #include #include #include @@ -83,16 +86,10 @@ void TransformPixelRow(png_structp ptr, png_row_infop row_info, void PNGReadPlatformFile(png_structp png, png_bytep buffer, png_size_t buffer_size) { -#if defined(STARBOARD) - // Casting between two pointer types. - base::PlatformFile file = - reinterpret_cast(png_get_io_ptr(png)); -#else - // Casting from a pointer to an int type. intptr_t temp = reinterpret_cast(png_get_io_ptr(png)); base::PlatformFile file = static_cast(temp); -#endif - int count = SbFileReadAll(file, reinterpret_cast(buffer), buffer_size); + int count = + starboard::ReadAll(file, reinterpret_cast(buffer), buffer_size); DCHECK_EQ(count, buffer_size); } diff --git a/cobalt/renderer/test/png_utils/png_encode.cc b/cobalt/renderer/test/png_utils/png_encode.cc index 507786a9bf4b..853c59940f71 100644 --- a/cobalt/renderer/test/png_utils/png_encode.cc +++ b/cobalt/renderer/test/png_utils/png_encode.cc @@ -58,7 +58,7 @@ void EncodeRGBAToPNG(const base::FilePath& png_file_path, base::File file(png_file_path, base::File::Flags::FLAG_OPEN_ALWAYS | base::File::Flags::FLAG_WRITE); - DCHECK_NE(file.GetPlatformFile(), kSbFileInvalid); + DCHECK_NE(file.GetPlatformFile(), -1); int bytes_written = file.WriteAtCurrentPos(reinterpret_cast(buffer.get()), size); base::RecordFileWriteStat(bytes_written); diff --git a/cobalt/speech/microphone_fake.cc b/cobalt/speech/microphone_fake.cc index 62f744af29ee..6da3197c6b8b 100644 --- a/cobalt/speech/microphone_fake.cc +++ b/cobalt/speech/microphone_fake.cc @@ -109,8 +109,7 @@ bool MicrophoneFake::Open() { DCHECK_NE(file_paths_.size(), 0u); size_t random_index = static_cast(base::RandGenerator(file_paths_.size())); - starboard::ScopedFile file(file_paths_[random_index].value().c_str(), - kSbFileOpenOnly | kSbFileRead, NULL, NULL); + starboard::ScopedFile file(file_paths_[random_index].value().c_str(), 0); DCHECK(file.IsValid()); int file_buffer_size = std::min(static_cast(file.GetSize()), kMaxBufferSize); diff --git a/cobalt/trace_event/json_file_outputter.cc b/cobalt/trace_event/json_file_outputter.cc index 4d1b2e1fa849..f3d10e329d8d 100644 --- a/cobalt/trace_event/json_file_outputter.cc +++ b/cobalt/trace_event/json_file_outputter.cc @@ -14,6 +14,9 @@ #include "cobalt/trace_event/json_file_outputter.h" +#include +#include + #include #include diff --git a/cobalt/watchdog/watchdog.cc b/cobalt/watchdog/watchdog.cc index 458ff320f08e..75c60f9358ac 100644 --- a/cobalt/watchdog/watchdog.cc +++ b/cobalt/watchdog/watchdog.cc @@ -193,7 +193,7 @@ void Watchdog::WriteWatchdogViolations() { base::JSONWriter::Write(*GetViolationsMap(), &watchdog_json); LOG(INFO) << "[Watchdog] Writing violations to JSON:\n" << watchdog_json; starboard::ScopedFile watchdog_file(GetWatchdogFilePath().c_str(), - kSbFileCreateAlways | kSbFileWrite); + O_CREAT | O_TRUNC | O_WRONLY); watchdog_file.WriteAll(watchdog_json.c_str(), static_cast(watchdog_json.size())); pending_write_ = false; diff --git a/cobalt/websocket/web_socket.h b/cobalt/websocket/web_socket.h index 2827dbb26476..2d59a6ceea01 100644 --- a/cobalt/websocket/web_socket.h +++ b/cobalt/websocket/web_socket.h @@ -37,6 +37,9 @@ #include "cobalt/web/message_event.h" #include "cobalt/websocket/web_socket_impl.h" +#undef close +#undef open + namespace cobalt { namespace websocket { diff --git a/net/proxy_resolution/proxy_config_service_linux.cc b/net/proxy_resolution/proxy_config_service_linux.cc index 3756a6b2938a..7a9085686784 100644 --- a/net/proxy_resolution/proxy_config_service_linux.cc +++ b/net/proxy_resolution/proxy_config_service_linux.cc @@ -870,7 +870,8 @@ Cobalt */ /* Cobalt base::ScopedFILE input(base::OpenFile(kioslaverc, "r")); Cobalt */ - base::ScopedFILE input(new starboard::ScopedFile(kioslaverc.value().c_str(), kSbFileOpenOnly | kSbFileRead)); + base::ScopedFILE input( + new starboard::ScopedFile(kioslaverc.value().c_str(), 0)); if (!input.get()) continue; diff --git a/starboard/common/file.h b/starboard/common/file.h index 06140080459c..15b8f4227850 100644 --- a/starboard/common/file.h +++ b/starboard/common/file.h @@ -22,7 +22,19 @@ #include "starboard/file.h" +#include +#include #include +#include + +#include "starboard/common/log.h" + +#ifdef _WIN32 +#undef open +#undef close +#define open sb_open +#define close sb_close +#endif namespace starboard { @@ -49,63 +61,56 @@ ssize_t WriteAll(int fd, const void* data, int size); // functions call the corresponding SbFile function. class ScopedFile { public: - ScopedFile(const char* path, - int flags, - bool* out_created, - SbFileError* out_error) - : file_(kSbFileInvalid) { - file_ = SbFileOpen(path, flags, out_created, out_error); + ScopedFile(const char* path, int flags, int mode) : file_(-1) { + file_ = open(path, flags, mode); } - ScopedFile(const char* path, int flags, bool* out_created) - : file_(kSbFileInvalid) { - file_ = SbFileOpen(path, flags, out_created, NULL); + ScopedFile(const char* path, int flags) : file_(-1) { + file_ = open(path, flags, S_IRUSR | S_IWUSR); } - ScopedFile(const char* path, int flags) : file_(kSbFileInvalid) { - file_ = SbFileOpen(path, flags, NULL, NULL); + ~ScopedFile() { + if (file_ >= 0) { + close(file_); + } } - ~ScopedFile() { SbFileClose(file_); } + int file() const { return file_; } - SbFile file() const { return file_; } - - bool IsValid() const { return SbFileIsValid(file_); } + bool IsValid() const { return file_ >= 0; } int64_t Seek(SbFileWhence whence, int64_t offset) const { - return SbFileSeek(file_, whence, offset); + return lseek(file_, static_cast(offset), static_cast(whence)); } - int Read(char* data, int size) const { return SbFileRead(file_, data, size); } + int Read(char* data, int size) const { return read(file_, data, size); } int ReadAll(char* data, int size) const { - return SbFileReadAll(file_, data, size); + return starboard::ReadAll(file_, data, size); } int Write(const char* data, int size) const { - int result = SbFileWrite(file_, data, size); + int result = write(file_, data, size); RecordFileWriteStat(result); return result; } int WriteAll(const char* data, int size) const { - int result = SbFileWriteAll(file_, data, size); + int result = starboard::WriteAll(file_, data, size); RecordFileWriteStat(result); return result; } - bool Truncate(int64_t length) const { return SbFileTruncate(file_, length); } + bool Truncate(int64_t length) const { return !ftruncate(file_, length); } - bool Flush() const { return SbFileFlush(file_); } + bool Flush() const { return !fsync(file_); } - bool GetInfo(SbFileInfo* out_info) const { - return SbFileGetInfo(file_, out_info); - } + bool GetInfo(struct stat* out_info) const { return !fstat(file_, out_info); } int64_t GetSize() const { - SbFileInfo file_info; + struct stat file_info; bool success = GetInfo(&file_info); - return (success ? file_info.size : -1); + return (success ? file_info.st_size : -1); } // disallow copy and move operations @@ -113,7 +118,7 @@ class ScopedFile { ScopedFile& operator=(const ScopedFile&) = delete; private: - SbFile file_; + int file_; }; } // namespace starboard diff --git a/starboard/loader_app/app_key_files.cc b/starboard/loader_app/app_key_files.cc index fcab813b9d66..d3224b0eb346 100644 --- a/starboard/loader_app/app_key_files.cc +++ b/starboard/loader_app/app_key_files.cc @@ -61,13 +61,14 @@ bool CreateAppKeyFile(const std::string& file_name_path) { if (file_name_path.empty()) { return false; } - SbFileError file_error = kSbFileOk; + starboard::ScopedFile file(file_name_path.c_str(), - kSbFileCreateAlways | kSbFileWrite, NULL, - &file_error); + O_CREAT | O_TRUNC | O_WRONLY); if (!file.IsValid()) { - SB_LOG(ERROR) << "Failed to open file: " << file_name_path - << "with error: " << file_error; + SB_LOG(ERROR) << "Failed to open file: " << file_name_path; + // TODO: retrieve error in starboard::ScopedFile + // SB_LOG(ERROR) << "Failed to open file: " << file_name_path + // << "with error: " << file.error_details(); return false; } return true; diff --git a/starboard/loader_app/drain_file_helper.cc b/starboard/loader_app/drain_file_helper.cc index a0f36227f0ec..3c25efbf704a 100644 --- a/starboard/loader_app/drain_file_helper.cc +++ b/starboard/loader_app/drain_file_helper.cc @@ -58,9 +58,7 @@ const std::string& ScopedDrainFile::path() const { } void ScopedDrainFile::CreateFile() { - SbFileError error = kSbFileOk; - starboard::ScopedFile file(path_.c_str(), kSbFileCreateOnly | kSbFileWrite, - NULL, &error); + starboard::ScopedFile file(path_.c_str(), O_CREAT | O_EXCL | O_WRONLY); EXPECT_TRUE(file.IsValid()); } diff --git a/starboard/loader_app/drain_file_test.cc b/starboard/loader_app/drain_file_test.cc index cb241ce04b96..2f23686a6f0f 100644 --- a/starboard/loader_app/drain_file_test.cc +++ b/starboard/loader_app/drain_file_test.cc @@ -244,9 +244,7 @@ TEST_F(DrainFileTest, SunnyDayPrepareDirectory) { path.append(kSbFileSepString); path.append(kAppKeyOne); - { - ScopedFile file(path.c_str(), kSbFileOpenAlways | kSbFileWrite, NULL, NULL); - } + { ScopedFile file(path.c_str(), O_CREAT | O_WRONLY); } EXPECT_TRUE(FileExists(path.c_str())); diff --git a/starboard/loader_app/installation_manager_test.cc b/starboard/loader_app/installation_manager_test.cc index ff6d02ac5f20..8e5a049a965b 100644 --- a/starboard/loader_app/installation_manager_test.cc +++ b/starboard/loader_app/installation_manager_test.cc @@ -233,10 +233,7 @@ TEST_P(InstallationManagerTest, Reset) { slot_path += kSbFileSepString; slot_path += "test_file"; created_files.push_back(slot_path); - SbFileError file_error = kSbFileOk; - starboard::ScopedFile file(slot_path.c_str(), - kSbFileCreateAlways | kSbFileWrite, NULL, - &file_error); + starboard::ScopedFile file(slot_path.c_str(), O_CREAT | O_TRUNC | O_WRONLY); ASSERT_TRUE(file.IsValid()); } ASSERT_EQ(IM_SUCCESS, ImReset()); diff --git a/starboard/loader_app/reset_evergreen_update_test.cc b/starboard/loader_app/reset_evergreen_update_test.cc index e37546be8348..3f922d60a717 100644 --- a/starboard/loader_app/reset_evergreen_update_test.cc +++ b/starboard/loader_app/reset_evergreen_update_test.cc @@ -45,10 +45,8 @@ TEST(ResetEvergreenUpdateTest, TestSunnyDayFile) { SB_LOG(INFO) << "file: " << file_path; { - SbFileError error; - ScopedFile file(file_path.c_str(), kSbFileOpenAlways | kSbFileWrite, - nullptr, &error); - ASSERT_EQ(kSbFileOk, error) << "Failed to open file for writing"; + ScopedFile file(file_path.c_str(), O_CREAT | O_WRONLY); + ASSERT_TRUE(file.IsValid()) << "Failed to open file for writing"; std::string data = "abc"; int bytes_written = file.WriteAll(data.c_str(), data.size()); } @@ -79,10 +77,8 @@ TEST(ResetEvergreenUpdateTest, TestSunnyDaySubdir) { SB_LOG(INFO) << "file: " << file_path; { - SbFileError error; - ScopedFile file(file_path.c_str(), kSbFileOpenAlways | kSbFileWrite, - nullptr, &error); - ASSERT_EQ(kSbFileOk, error) << "Failed to open file for writing"; + ScopedFile file(file_path.c_str(), O_CREAT | O_WRONLY); + ASSERT_TRUE(file.IsValid()) << "Failed to open file for writing"; std::string data = "abc"; int bytes_written = file.WriteAll(data.c_str(), data.size()); } diff --git a/starboard/nplb/nplb_evergreen_compat_tests/storage_test.cc b/starboard/nplb/nplb_evergreen_compat_tests/storage_test.cc index ce0179dde0cf..91f47b172e25 100644 --- a/starboard/nplb/nplb_evergreen_compat_tests/storage_test.cc +++ b/starboard/nplb/nplb_evergreen_compat_tests/storage_test.cc @@ -45,17 +45,15 @@ class StorageTest : public ::testing::Test { void WriteBuffer(const char* file_path, const char* buffer, size_t buffer_size) { - SbFileError error; - ScopedFile file(file_path, kSbFileOpenAlways | kSbFileWrite, nullptr, &error); - ASSERT_EQ(kSbFileOk, error) << "Failed to open file for writing"; + ScopedFile file(file_path, O_CREAT | O_WRONLY); + ASSERT_TRUE(file.IsValid()) << "Failed to open file for writing"; int bytes_written = file.WriteAll(buffer, buffer_size); ASSERT_EQ(kBufSize, bytes_written); } void ReadBuffer(const char* file_path, char* buffer, size_t buffer_size) { - SbFileError error; - ScopedFile file(file_path, kSbFileOpenOnly | kSbFileRead, nullptr, &error); - ASSERT_EQ(kSbFileOk, error) << "Failed to open file for reading"; + ScopedFile file(file_path, 0); + ASSERT_TRUE(file.IsValid()) << "Failed to open file for reading"; int count = file.ReadAll(buffer, buffer_size); ASSERT_EQ(kBufSize, count); } diff --git a/starboard/nplb/posix_compliance/posix_socket_send_test.cc b/starboard/nplb/posix_compliance/posix_socket_send_test.cc index 561b72b2021c..b057d22a3f1f 100644 --- a/starboard/nplb/posix_compliance/posix_socket_send_test.cc +++ b/starboard/nplb/posix_compliance/posix_socket_send_test.cc @@ -104,7 +104,7 @@ TEST(PosixSocketSendTest, RainyDaySendToClosedSocket) { pthread_create(&send_thread, NULL, PosixSocketSendToServerSocketEntryPoint, static_cast(&trio_as_void_ptr)); - // Close the client, which should cause writes to the server socket to fail. + // Close the client, which should cause writes to the server socket to EXPECT_TRUE(close(client_socket_fd) == 0); // Wait for the thread to exit and check the last socket error. diff --git a/starboard/nplb/posix_compliance/posix_socket_sendto_test.cc b/starboard/nplb/posix_compliance/posix_socket_sendto_test.cc index 5ff829f2239a..762352383166 100644 --- a/starboard/nplb/posix_compliance/posix_socket_sendto_test.cc +++ b/starboard/nplb/posix_compliance/posix_socket_sendto_test.cc @@ -107,7 +107,7 @@ TEST(PosixSocketSendtoTest, RainyDaySendToClosedSocket) { pthread_create(&send_thread, NULL, PosixSocketSendToServerSocketEntryPoint, static_cast(&trio_as_void_ptr)); - // Close the client, which should cause writes to the server socket to fail. + // Close the client, which should cause writes to the server socket to EXPECT_TRUE(close(client_socket_fd) == 0); // Wait for the thread to exit and check the last socket error. diff --git a/starboard/nplb/system_get_path_test.cc b/starboard/nplb/system_get_path_test.cc index 2a7dd97f74ea..bced37c3251e 100644 --- a/starboard/nplb/system_get_path_test.cc +++ b/starboard/nplb/system_get_path_test.cc @@ -176,8 +176,8 @@ TEST(SbSystemGetPathTest, CanWriteAndReadCache) { // Write to the file and check that we can read from it. std::string content_to_write = "test content"; { - starboard::ScopedFile test_file_writer( - path.data(), kSbFileCreateAlways | kSbFileWrite, NULL, NULL); + starboard::ScopedFile test_file_writer(path.data(), + O_CREAT | O_TRUNC | O_WRONLY); EXPECT_GT( test_file_writer.WriteAll(content_to_write.c_str(), static_cast(content_to_write.size())), @@ -191,8 +191,7 @@ TEST(SbSystemGetPathTest, CanWriteAndReadCache) { const int kBufferLength = 16 * 1024; char content_read[kBufferLength] = {0}; { - starboard::ScopedFile test_file_reader( - path.data(), kSbFileOpenOnly | kSbFileRead, NULL, NULL); + starboard::ScopedFile test_file_reader(path.data(), 0); EXPECT_GT(test_file_reader.ReadAll(content_read, kFileSize), 0); } EXPECT_EQ(content_read, content_to_write); diff --git a/starboard/shared/linux/system_get_used_cpu_memory.cc b/starboard/shared/linux/system_get_used_cpu_memory.cc index a34ab6508592..b7ddc0f35f9b 100644 --- a/starboard/shared/linux/system_get_used_cpu_memory.cc +++ b/starboard/shared/linux/system_get_used_cpu_memory.cc @@ -73,8 +73,7 @@ int64_t SearchForMemoryValue(const char* search_key, const char* buffer) { int64_t SbSystemGetUsedCPUMemory() { // Read our process' current physical memory usage from /proc/self/status. - starboard::ScopedFile status_file("/proc/self/status", - kSbFileOpenOnly | kSbFileRead); + starboard::ScopedFile status_file("/proc/self/status", 0); if (!status_file.IsValid()) { SB_LOG(ERROR) << "Error opening /proc/self/status in order to query self memory " diff --git a/starboard/shared/starboard/link_receiver.cc b/starboard/shared/starboard/link_receiver.cc index 6ac8b323f672..1c22014c736e 100644 --- a/starboard/shared/starboard/link_receiver.cc +++ b/starboard/shared/starboard/link_receiver.cc @@ -39,12 +39,14 @@ namespace { scoped_ptr CreateServerSocket(SbSocketAddressType address_type) { scoped_ptr socket(new Socket(address_type)); if (!socket->IsValid()) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "SbSocketCreate failed"; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "SbSocketCreate failed"; return scoped_ptr().Pass(); } if (!socket->SetReuseAddress(true)) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "SbSocketSetReuseAddress failed"; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "SbSocketSetReuseAddress failed"; return scoped_ptr().Pass(); } @@ -67,8 +69,8 @@ scoped_ptr CreateLocallyBoundSocket(SbSocketAddressType address_type, } SbSocketError result = socket->Bind(&address); if (result != kSbSocketOk) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "SbSocketBind to " << port - << " failed: " << result; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "SbSocketBind to " << port << " failed: " << result; return scoped_ptr().Pass(); } @@ -115,7 +117,8 @@ std::string GetTemporaryDirectory() { bool has_temp = SbSystemGetPath(kSbSystemPathTempDirectory, temp_path.get(), kMaxPathLength); if (!has_temp) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "No temporary directory."; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "No temporary directory."; return ""; } @@ -131,9 +134,10 @@ void CreateTemporaryFile(const char* name, const char* contents, int size) { path += kSbFileSepString; path += name; - ScopedFile file(path.c_str(), kSbFileCreateAlways | kSbFileWrite); + ScopedFile file(path.c_str(), O_CREAT | O_TRUNC | O_WRONLY); if (!file.IsValid()) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "Unable to create: " << path; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "Unable to create: " << path; return; } @@ -316,7 +320,8 @@ bool LinkReceiver::Impl::AddForAccept(Socket* socket) { if (!SbSocketWaiterAdd(waiter_, socket->socket(), this, &LinkReceiver::Impl::HandleAccept, kSbSocketWaiterInterestRead, true)) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "SbSocketWaiterAdd failed."; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "SbSocketWaiterAdd failed."; return false; } return true; @@ -326,7 +331,8 @@ bool LinkReceiver::Impl::AddForRead(Connection* connection) { if (!SbSocketWaiterAdd(waiter_, connection->socket->socket(), this, &LinkReceiver::Impl::HandleRead, kSbSocketWaiterInterestRead, false)) { - SB_LOG(ERROR) << __FUNCTION__ << ": " << "SbSocketWaiterAdd failed."; + SB_LOG(ERROR) << __FUNCTION__ << ": " + << "SbSocketWaiterAdd failed."; return false; } return true; diff --git a/starboard/shared/starboard/localized_strings.cc b/starboard/shared/starboard/localized_strings.cc index 838c4e4f4e0b..ab9a9fe71aa4 100644 --- a/starboard/shared/starboard/localized_strings.cc +++ b/starboard/shared/starboard/localized_strings.cc @@ -44,30 +44,30 @@ bool ReadFile(const std::string& filename, std::string* out_result) { SB_DCHECK(filename.length() > 0); SB_DCHECK(out_result); - ScopedFile file(filename.c_str(), kSbFileOpenOnly | kSbFileRead); + ScopedFile file(filename.c_str(), O_RDONLY); if (!file.IsValid()) { SB_DLOG(WARNING) << "Cannot open i18n file: " << filename; return false; } - SbFileInfo file_info = {0}; + struct stat file_info; bool got_info = file.GetInfo(&file_info); if (!got_info) { SB_DLOG(ERROR) << "Cannot get information for i18n file."; return false; } - SB_DCHECK(file_info.size > 0); + SB_DCHECK(file_info.st_size > 0); const int kMaxBufferSize = 16 * 1024; - if (file_info.size > kMaxBufferSize) { - SB_DLOG(ERROR) << "i18n file exceeds maximum size: " << file_info.size + if (file_info.st_size > kMaxBufferSize) { + SB_DLOG(ERROR) << "i18n file exceeds maximum size: " << file_info.st_size << " (" << kMaxBufferSize << ")"; return false; } - char* buffer = new char[file_info.size]; + char* buffer = new char[file_info.st_size]; SB_DCHECK(buffer); - int64_t bytes_to_read = file_info.size; + int64_t bytes_to_read = file_info.st_size; char* buffer_pos = buffer; while (bytes_to_read > 0) { int max_bytes_to_read = static_cast( @@ -82,7 +82,7 @@ bool ReadFile(const std::string& filename, std::string* out_result) { buffer_pos += bytes_read; } - *out_result = std::string(buffer, file_info.size); + *out_result = std::string(buffer, file_info.st_size); delete[] buffer; return true; } @@ -125,6 +125,7 @@ LocalizedStrings::MatchType LocalizedStrings::GetMatchType() const { bool LocalizedStrings::LoadStrings(const std::string& language) { const std::string filename = GetFilenameForLanguage(language); + std::string file_contents; if (!ReadFile(filename, &file_contents)) { SB_DLOG(ERROR) << "Error reading i18n file."; diff --git a/starboard/shared/starboard/player/file_cache_reader.cc b/starboard/shared/starboard/player/file_cache_reader.cc index d5c84093ac8c..bce8ddec522d 100644 --- a/starboard/shared/starboard/player/file_cache_reader.cc +++ b/starboard/shared/starboard/player/file_cache_reader.cc @@ -84,8 +84,7 @@ void FileCacheReader::EnsureFileOpened() { if (file_) { return; } - file_.reset( - new ScopedFile(absolute_path_.c_str(), kSbFileOpenOnly | kSbFileRead)); + file_.reset(new ScopedFile(absolute_path_.c_str(), 0)); SB_CHECK(file_->IsValid()); max_file_cache_size_ = diff --git a/starboard/shared/starboard/player/filter/testing/file_cache_reader_test.cc b/starboard/shared/starboard/player/filter/testing/file_cache_reader_test.cc index 007128b29f75..c486ff7e42ce 100644 --- a/starboard/shared/starboard/player/filter/testing/file_cache_reader_test.cc +++ b/starboard/shared/starboard/player/filter/testing/file_cache_reader_test.cc @@ -49,8 +49,7 @@ TEST_F(FileCacheReaderTest, FileCacheReader) { { // Use a 5MB snippet from the true file to compare against. const size_t kTestSize = 5 * 1024 * 1024; - ScopedFile file(file_cache_reader_.GetAbsolutePathName().c_str(), - kSbFileOpenOnly | kSbFileRead); + ScopedFile file(file_cache_reader_.GetAbsolutePathName().c_str(), 0); true_contents.resize(kTestSize); int bytes_read = file.ReadAll(true_contents.data(), kTestSize); SB_CHECK(bytes_read == kTestSize); diff --git a/starboard/shared/uwp/log_writer_win32.cc b/starboard/shared/uwp/log_writer_win32.cc index 3e12b7e8a390..7d6f5a8bdf6e 100644 --- a/starboard/shared/uwp/log_writer_win32.cc +++ b/starboard/shared/uwp/log_writer_win32.cc @@ -31,12 +31,9 @@ namespace { class LogWriterWin32 : public ILogWriter { public: explicit LogWriterWin32(const std::string& file_path) { - SbFileError out_error = kSbFileOk; - bool created_ok = false; - file_.reset(new ScopedFile(file_path.c_str(), - kSbFileCreateAlways | kSbFileWrite, &created_ok, - &out_error)); - if (!created_ok || out_error != kSbFileOk) { + file_.reset( + new ScopedFile(file_path.c_str(), O_CREAT | O_TRUNC | O_WRONLY)); + if (!file_->IsValid()) { SB_LOG(ERROR) << "Could not create watchdog file " << file_path; file_.reset(); } diff --git a/starboard/shared/widevine/widevine_storage.cc b/starboard/shared/widevine/widevine_storage.cc index d6c8a3daae2a..3bd95360390a 100644 --- a/starboard/shared/widevine/widevine_storage.cc +++ b/starboard/shared/widevine/widevine_storage.cc @@ -32,7 +32,7 @@ namespace { void ReadFile(const std::string& path_name, std::vector* content) { SB_DCHECK(content); - ScopedFile file(path_name.c_str(), kSbFileOpenOnly | kSbFileRead); + ScopedFile file(path_name.c_str(), 0); if (!file.IsValid()) { content->clear(); @@ -60,7 +60,7 @@ void ReadFile(const std::string& path_name, std::vector* content) { bool WriteFile(const std::string& path_name, const std::vector& content) { - ScopedFile file(path_name.c_str(), kSbFileCreateAlways | kSbFileWrite); + ScopedFile file(path_name.c_str(), O_CREAT | O_TRUNC | O_WRONLY); if (!file.IsValid()) { SB_LOG(INFO) << "Failed to create " << path_name << " for writing."; diff --git a/starboard/shared/win32/posix_emu/include/fcntl.h b/starboard/shared/win32/posix_emu/include/fcntl.h index 93f9be37b70d..a86989dfa0e5 100644 --- a/starboard/shared/win32/posix_emu/include/fcntl.h +++ b/starboard/shared/win32/posix_emu/include/fcntl.h @@ -20,7 +20,6 @@ #include // Needed for `open`, which is in fcntl.h on POSIX #undef open -#undef close // in unistd.h on POSIX, and handles both files and sockets #define F_GETFL 3 /* get file->f_flags */ #define F_SETFL 4 /* set file->f_flags */ @@ -36,6 +35,9 @@ extern "C" { int sb_fcntl(int fd, int cmd, ... /*arg*/); #define fcntl sb_fcntl +int sb_open(const char* path, int oflag, ...); +#define open sb_open + #ifdef __cplusplus } #endif @@ -62,6 +64,4 @@ static const mode_t S_IXOTH = 0x00010000; static const mode_t MS_MODE_MASK = 0x0000ffff; -int open(const char* path, int oflag, ...); - #endif // STARBOARD_SHARED_WIN32_POSIX_EMU_INCLUDE_FCNTL_H_ diff --git a/starboard/shared/win32/posix_emu/include/unistd.h b/starboard/shared/win32/posix_emu/include/unistd.h index 47a272714079..9b6a8b58b5a3 100644 --- a/starboard/shared/win32/posix_emu/include/unistd.h +++ b/starboard/shared/win32/posix_emu/include/unistd.h @@ -15,20 +15,23 @@ #ifndef STARBOARD_SHARED_WIN32_POSIX_EMU_INCLUDE_UNISTD_H_ #define STARBOARD_SHARED_WIN32_POSIX_EMU_INCLUDE_UNISTD_H_ +#include #include +#undef close + #ifdef __cplusplus extern "C" { #endif -// The implementation of the following functions are located in socket.cc. +int sb_close(int fd); +#define close sb_close -// This function will handle both files and sockets. -int close(int fd); +// The implementation of the following functions are located in socket.cc. int fsync(int fd); -int ftruncate(int fd, off_t length); +int ftruncate(int fd, int64_t length); long lseek(int fd, long offset, int origin); // NOLINT diff --git a/starboard/shared/win32/posix_emu/socket.cc b/starboard/shared/win32/posix_emu/socket.cc index f1d160f6ed88..ca2539b096bc 100644 --- a/starboard/shared/win32/posix_emu/socket.cc +++ b/starboard/shared/win32/posix_emu/socket.cc @@ -27,6 +27,9 @@ #include "starboard/common/log.h" #include "starboard/types.h" +#undef open +#undef close + static int gen_fd() { static int fd = 100; fd++; @@ -249,7 +252,7 @@ int sb_socket(int domain, int type, int protocol) { return handle_db_put(handle); } -int open(const char* path, int oflag, ...) { +int sb_open(const char* path, int oflag, ...) { va_list args; va_start(args, oflag); int fd; @@ -272,7 +275,7 @@ int open(const char* path, int oflag, ...) { return handle_db_put(handle); } -int close(int fd) { +int sb_close(int fd) { FileOrSocket handle = handle_db_get(fd, true); if (!handle.is_file && handle.socket == INVALID_SOCKET) { @@ -297,7 +300,7 @@ int fsync(int fd) { return _commit(handle.file); } -int ftruncate(int fd, off_t length) { +int ftruncate(int fd, int64_t length) { FileOrSocket handle = handle_db_get(fd, false); if (!handle.is_file) { return -1; @@ -318,6 +321,7 @@ int sb_fstat(int fd, struct stat* buffer) { if (!handle.is_file) { return -1; } + return _fstat(handle.file, (struct _stat*)buffer); } diff --git a/starboard/tools/api_leak_detector/api_leak_detector.py b/starboard/tools/api_leak_detector/api_leak_detector.py index 15bcfe76960e..7f515e52ecad 100755 --- a/starboard/tools/api_leak_detector/api_leak_detector.py +++ b/starboard/tools/api_leak_detector/api_leak_detector.py @@ -129,6 +129,7 @@ 'read', 'sched_yield', 'stat', + 'open', 'pthread_attr_init', 'pthread_attr_destroy', 'pthread_attr_getdetachstate', diff --git a/starboard/win/win32/test_filters.py b/starboard/win/win32/test_filters.py index dcd4664e289f..b895868c555f 100644 --- a/starboard/win/win32/test_filters.py +++ b/starboard/win/win32/test_filters.py @@ -20,6 +20,10 @@ # pylint: disable=line-too-long _FILTERED_TESTS = { 'nplb': [ + # These tests hang too long on Windows. + 'PosixSocketSendTest.RainyDaySendToClosedSocket', + 'PosixSocketSendtoTest.RainyDaySendToClosedSocket', + # Currently frequently flaky on Windows 'SbConditionVariableWaitTimedTest.FLAKY_SunnyDayAutoInit', 'PosixConditionVariableWaitTimedTest.FLAKY_SunnyDayAutoInit', diff --git a/third_party/googletest/src/googletest/src/gtest.cc b/third_party/googletest/src/googletest/src/gtest.cc index d2b9fa7c5b6d..05dac7aea349 100644 --- a/third_party/googletest/src/googletest/src/gtest.cc +++ b/third_party/googletest/src/googletest/src/gtest.cc @@ -4189,11 +4189,8 @@ class XmlUnitTestResultPrinter : public EmptyTestEventListener { #if GTEST_OS_STARBOARD void WriteOuputFile(const std::string &output_file, const std::string &data) { - SbFileError err; - starboard::ScopedFile cache_file( - output_file.c_str(), kSbFileCreateAlways | kSbFileWrite, NULL, &err); - // TODO: Change to SB_DCHECK once all platforms are verified - if(err != kSbFileOk) { + starboard::ScopedFile cache_file(output_file.c_str(), O_CREAT | O_TRUNC | O_WRONLY); + if (!cache_file.IsValid()) { SB_LOG(ERROR) << "Unable to open file " << output_file << " for XML output"; } cache_file.WriteAll(data.c_str(), static_cast(data.size())); diff --git a/third_party/musl/src/starboard/sys/stat.c b/third_party/musl/src/starboard/sys/stat.c index 6a32915f42ea..a41d4fe2d39f 100644 --- a/third_party/musl/src/starboard/sys/stat.c +++ b/third_party/musl/src/starboard/sys/stat.c @@ -1,5 +1,6 @@ #if SB_API_VERSION < 16 +#include #include #include @@ -27,6 +28,10 @@ static SB_C_FORCE_INLINE time_t WindowsUsecToTimeT(int64_t time) { // SbDirectoryCanOpen, SbFileGetPathInfo, SbFileExists, SbFileCanOpen int stat(const char *path, struct stat *file_info) { + if(!SbFileExists(path)) { + errno = ENOENT; + } + SbFileInfo out_info; if (!SbFileGetPathInfo(path, &out_info)){ return -1;