From e019703fecc5625ff689b1142a9d4a526426a541 Mon Sep 17 00:00:00 2001 From: cobalt-github-releaser-bot <95661244+cobalt-github-releaser-bot@users.noreply.github.com> Date: Tue, 27 Aug 2024 15:56:54 -0700 Subject: [PATCH] Cherry pick PR #4012: Deprecate SbFile, SbFileError, SbFileInfo types. (#4042) Refer to the original PR: https://github.com/youtube/cobalt/pull/4012 b/302715109 --------- Co-authored-by: Yijia Zhang <45114178+yjzhang111@users.noreply.github.com> --- base/files/file.h | 4 - base/files/file_starboard.cc | 15 +-- base/memory/platform_shared_memory_handle.h | 2 +- .../trust_store_in_memory_starboard.cc | 1 - .../posix_directory_open_test.cc | 1 - starboard/shared/win32/file_internal.cc | 106 ++++++++++++++++++ starboard/shared/win32/file_internal.h | 3 + starboard/shared/win32/log_file_impl.cc | 5 +- starboard/shared/win32/minidump.cc | 13 +-- starboard/shared/win32/posix_emu/dirent.cc | 6 +- third_party/libjpeg_turbo/BUILD.gn | 1 + third_party/libjpeg_turbo/cdjpeg.h | 10 +- third_party/libjpeg_turbo/jmemsys.h | 2 +- third_party/libjpeg_turbo/jpeglib.h | 8 +- 14 files changed, 136 insertions(+), 41 deletions(-) diff --git a/base/files/file.h b/base/files/file.h index 855f763e8d9e..e1b57553f94f 100644 --- a/base/files/file.h +++ b/base/files/file.h @@ -433,10 +433,6 @@ class BASE_EXPORT File { #endif }; -#if defined(STARBOARD) -void SetLastFileError(File::Error error); -#endif - } // namespace base #endif // BASE_FILES_FILE_H_ diff --git a/base/files/file_starboard.cc b/base/files/file_starboard.cc index 17a4a4d308c1..393c6b5d095b 100644 --- a/base/files/file_starboard.cc +++ b/base/files/file_starboard.cc @@ -32,15 +32,6 @@ namespace base { -namespace { -SbFileError g_sb_file_error = kSbFileOk; -} // namespace - -// TODO: remove SbFileError -void SetLastFileError(File::Error error) { - g_sb_file_error = static_cast(error); -} - void RecordFileWriteStat(int write_file_result) { auto& stats_tracker = starboard::StatsTrackerContainer::GetInstance()->stats_tracker(); @@ -53,9 +44,9 @@ void RecordFileWriteStat(int write_file_result) { } // Make sure our Whence mappings match the system headers. -static_assert(File::FROM_BEGIN == static_cast(kSbFileFromBegin) && - File::FROM_CURRENT == static_cast(kSbFileFromCurrent) && - File::FROM_END == static_cast(kSbFileFromEnd), +static_assert(File::FROM_BEGIN == static_cast(SEEK_SET) && + File::FROM_CURRENT == static_cast(SEEK_CUR) && + File::FROM_END == static_cast(SEEK_END), "Whence enums from base must match those of Starboard."); void File::Info::FromStat(const stat_wrapper_t& stat_info) { diff --git a/base/memory/platform_shared_memory_handle.h b/base/memory/platform_shared_memory_handle.h index e7484cabc6a9..2869f0313a41 100644 --- a/base/memory/platform_shared_memory_handle.h +++ b/base/memory/platform_shared_memory_handle.h @@ -54,7 +54,7 @@ struct BASE_EXPORT ScopedFDPair { // Platform-specific shared memory type used by the shared memory system. #if defined(STARBOARD) -using PlatformSharedMemoryHandle = SbFile; +using PlatformSharedMemoryHandle = int; using ScopedPlatformSharedMemoryHandle = ScopedFD; #elif BUILDFLAG(IS_APPLE) using PlatformSharedMemoryHandle = mach_port_t; diff --git a/net/cert/internal/trust_store_in_memory_starboard.cc b/net/cert/internal/trust_store_in_memory_starboard.cc index 0b48d7ad1b30..1bc400f5ce53 100644 --- a/net/cert/internal/trust_store_in_memory_starboard.cc +++ b/net/cert/internal/trust_store_in_memory_starboard.cc @@ -118,7 +118,6 @@ std::shared_ptr TrustStoreInMemoryStarboard::TryLoadCer return nullptr; } - SbFileError out_error; char cert_buffer[kCertBufferSize]; base::FilePath cert_path = GetCertificateDirPath().Append(cert_file_name); base::File cert_file(cert_path, base::File::Flags::FLAG_OPEN | base::File::Flags::FLAG_READ); diff --git a/starboard/nplb/posix_compliance/posix_directory_open_test.cc b/starboard/nplb/posix_compliance/posix_directory_open_test.cc index eba5e7918027..d979dbcb4a06 100644 --- a/starboard/nplb/posix_compliance/posix_directory_open_test.cc +++ b/starboard/nplb/posix_compliance/posix_directory_open_test.cc @@ -94,7 +94,6 @@ TEST(PosixDirectoryOpenTest, FailsInvalidPath) { struct stat info; ASSERT_FALSE(stat(path.c_str(), &info) == 0); - SbFileError error = kSbFileErrorMax; DIR* directory = opendir(path.c_str()); EXPECT_FALSE(directory != NULL); if (directory != NULL) { diff --git a/starboard/shared/win32/file_internal.cc b/starboard/shared/win32/file_internal.cc index de07d48bd657..fdc4b5ae58b8 100644 --- a/starboard/shared/win32/file_internal.cc +++ b/starboard/shared/win32/file_internal.cc @@ -14,6 +14,8 @@ #include "starboard/shared/win32/file_internal.h" +#include +#include #include #include @@ -23,6 +25,8 @@ #include "starboard/shared/win32/error_utils.h" #include "starboard/shared/win32/wchar_utils.h" +#define O_ACCMODE (O_RDONLY | O_WRONLY | O_RDWR) + namespace sbwin32 = starboard::shared::win32; namespace starboard { @@ -81,6 +85,108 @@ std::wstring NormalizeWin32Path(std::wstring str) { return NormalizePathSeparator(str); } +HANDLE OpenFileOrDirectory(const char* path, int flags) { + // Note that FILE_SHARE_DELETE allows a file to be deleted while there + // are other handles open for read/write. This is necessary for the + // Async file tests which, due to system timing, will sometimes have + // outstanding handles open and fail to delete, failing the test. + const DWORD share_mode = + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; + + DWORD desired_access = 0; + if ((flags & O_ACCMODE) == O_RDONLY) { + desired_access |= GENERIC_READ; + } else if ((flags & O_ACCMODE) == O_WRONLY) { + desired_access |= GENERIC_WRITE; + flags &= ~O_WRONLY; + } else if ((flags & O_ACCMODE) == O_RDWR) { + desired_access |= GENERIC_READ | GENERIC_WRITE; + flags &= ~O_RDWR; + } else { + // Applications shall specify exactly one of the first three file access + // modes. + errno = EINVAL; + return INVALID_HANDLE_VALUE; + } + + DWORD creation_disposition = 0; + if (!flags) { + creation_disposition = OPEN_EXISTING; + } + + if (flags & O_CREAT && flags & O_EXCL) { + SB_DCHECK(!creation_disposition); + creation_disposition = CREATE_NEW; + flags &= ~(O_CREAT | O_EXCL); + } + + if (flags & O_CREAT && flags & O_TRUNC) { + SB_DCHECK(!creation_disposition); + creation_disposition = CREATE_ALWAYS; + flags &= ~(O_CREAT | O_TRUNC); + } + + if (flags & O_TRUNC) { + SB_DCHECK(!creation_disposition); + SB_DCHECK((flags & O_WRONLY) || (flags & O_RDWR)); + creation_disposition = TRUNCATE_EXISTING; + flags &= ~O_TRUNC; + } + + if (flags & O_CREAT) { + SB_DCHECK(!creation_disposition); + creation_disposition = OPEN_ALWAYS; + flags &= ~O_CREAT; + } + + // SbFileOpen does not support any other combination of flags. + if (flags || !creation_disposition) { + errno = ENOTSUP; + return INVALID_HANDLE_VALUE; + } + + SB_DCHECK(desired_access != 0) << "Invalid permission flag."; + + std::wstring path_wstring = NormalizeWin32Path(path); + CREATEFILE2_EXTENDED_PARAMETERS create_ex_params = {0}; + // Enabling |FILE_FLAG_BACKUP_SEMANTICS| allows us to figure out if the path + // is a directory. + create_ex_params.dwFileFlags = FILE_FLAG_BACKUP_SEMANTICS; + create_ex_params.dwFileFlags |= FILE_FLAG_POSIX_SEMANTICS; + create_ex_params.dwFileAttributes = FILE_ATTRIBUTE_NORMAL; + create_ex_params.dwSecurityQosFlags = SECURITY_ANONYMOUS; + create_ex_params.dwSize = sizeof(CREATEFILE2_EXTENDED_PARAMETERS); + + HANDLE file_handle = + CreateFile2(path_wstring.c_str(), desired_access, share_mode, + creation_disposition, &create_ex_params); + + const DWORD last_error = GetLastError(); + + if (!starboard::shared::win32::IsValidHandle(file_handle)) { + switch (last_error) { + case ERROR_ACCESS_DENIED: + errno = EACCES; + break; + case ERROR_FILE_EXISTS: { + if (creation_disposition == CREATE_NEW) { + errno = EEXIST; + } else { + errno = EPERM; + } + break; + } + case ERROR_FILE_NOT_FOUND: + errno = ENOENT; + break; + default: + errno = EPERM; + } + } + + return file_handle; +} + HANDLE OpenFileOrDirectory(const char* path, int flags, bool* out_created, diff --git a/starboard/shared/win32/file_internal.h b/starboard/shared/win32/file_internal.h index 9f60f69fd2c1..c3d42c0776f0 100644 --- a/starboard/shared/win32/file_internal.h +++ b/starboard/shared/win32/file_internal.h @@ -57,6 +57,7 @@ class SbFilePrivate { SbFilePrivate(const SbFilePrivate&) = delete; SbFilePrivate& operator=(const SbFilePrivate&) = delete; }; + #pragma warning(pop) namespace starboard { @@ -87,6 +88,8 @@ inline bool PathEndsWith(const std::wstring& path, const wchar_t* filename) { std::wstring NormalizeWin32Path(std::string str); std::wstring NormalizeWin32Path(std::wstring str); +HANDLE OpenFileOrDirectory(const char* path, int flags); + HANDLE OpenFileOrDirectory(const char* path, int flags, bool* out_created, diff --git a/starboard/shared/win32/log_file_impl.cc b/starboard/shared/win32/log_file_impl.cc index 9158e6e70ec4..6932dec04f6d 100644 --- a/starboard/shared/win32/log_file_impl.cc +++ b/starboard/shared/win32/log_file_impl.cc @@ -85,11 +85,10 @@ void OpenLogInCacheDirectory(const char* log_file_name, int creation_flags) { } void OpenLogFile(const char* path, const int creation_flags) { - SB_DCHECK((creation_flags & kSbFileOpenAlways) || - (creation_flags & kSbFileCreateAlways)); + SB_DCHECK(creation_flags & O_CREAT); SB_DLOG(INFO) << "Logging to [" << path << "]"; - int flags = creation_flags | kSbFileWrite; + int flags = creation_flags | O_WRONLY; pthread_mutex_lock(&log_mutex); CloseLogFileWithoutLock(); diff --git a/starboard/shared/win32/minidump.cc b/starboard/shared/win32/minidump.cc index 916eb106a253..b236269dcdd0 100644 --- a/starboard/shared/win32/minidump.cc +++ b/starboard/shared/win32/minidump.cc @@ -19,6 +19,7 @@ // clang-format on #include #include +#include #include #include @@ -72,16 +73,12 @@ class DumpHandler { SbLogRaw("Could not write minidump because the dump path is missing."); return; } - bool out_created = false; - SbFileError out_error = kSbFileOk; - HANDLE file_handle = OpenFileOrDirectory(file_path_.c_str(), - kSbFileCreateAlways | kSbFileWrite, - &out_created, &out_error); + HANDLE file_handle = + OpenFileOrDirectory(file_path_.c_str(), O_CREAT | O_TRUNC | O_WRONLY); - const bool file_ok = out_created && (out_error == kSbFileOk) && - (file_handle != NULL) && - (file_handle != INVALID_HANDLE_VALUE); + const bool file_ok = + (file_handle != NULL) && (file_handle != INVALID_HANDLE_VALUE); if (!file_ok) { std::stringstream ss; diff --git a/starboard/shared/win32/posix_emu/dirent.cc b/starboard/shared/win32/posix_emu/dirent.cc index 6db9e3cc68ec..e2531a8017e2 100644 --- a/starboard/shared/win32/posix_emu/dirent.cc +++ b/starboard/shared/win32/posix_emu/dirent.cc @@ -14,6 +14,7 @@ #include +#include #include #include @@ -163,9 +164,8 @@ DIR* opendir(const char* path) { return nullptr; } - SbFileError out_error; - HANDLE directory_handle = starboard::shared::win32::OpenFileOrDirectory( - path, kSbFileOpenOnly | kSbFileRead, nullptr, &out_error); + HANDLE directory_handle = + starboard::shared::win32::OpenFileOrDirectory(path, O_RDONLY); if (!starboard::shared::win32::IsValidHandle(directory_handle)) { errno = EBADF; diff --git a/third_party/libjpeg_turbo/BUILD.gn b/third_party/libjpeg_turbo/BUILD.gn index 7b15c9a7ec80..2998236a723c 100644 --- a/third_party/libjpeg_turbo/BUILD.gn +++ b/third_party/libjpeg_turbo/BUILD.gn @@ -24,6 +24,7 @@ source_set("libjpeg_headers") { public_deps = [ "//starboard:starboard_headers_only", "//starboard/common", + "//starboard/common:file_wrapper", ] } } diff --git a/third_party/libjpeg_turbo/cdjpeg.h b/third_party/libjpeg_turbo/cdjpeg.h index 86e47c905374..22099e1e8231 100644 --- a/third_party/libjpeg_turbo/cdjpeg.h +++ b/third_party/libjpeg_turbo/cdjpeg.h @@ -33,7 +33,7 @@ struct cjpeg_source_struct { void (*finish_input) (j_compress_ptr cinfo, cjpeg_source_ptr sinfo); #if defined(STARBOARD) - SbFile *input_file; + FileStruct *input_file; #else FILE *input_file; #endif @@ -73,7 +73,7 @@ struct djpeg_dest_struct { /* Target file spec; filled in by djpeg.c after object is created. */ #if defined(STARBOARD) - SbFile *output_file; + FileStruct *output_file; #else FILE *output_file; #endif @@ -135,7 +135,7 @@ EXTERN(boolean) set_sample_factors(j_compress_ptr cinfo, char *arg); /* djpeg support routines (in rdcolmap.c) */ #if defined(STARBOARD) -EXTERN(void) read_color_map(j_decompress_ptr cinfo, SbFile *infile); +EXTERN(void) read_color_map(j_decompress_ptr cinfo, FileStruct *infile); #else EXTERN(void) read_color_map(j_decompress_ptr cinfo, FILE *infile); #endif @@ -148,8 +148,8 @@ EXTERN(void) end_progress_monitor(j_common_ptr cinfo); EXTERN(boolean) keymatch(char *arg, const char *keyword, int minchars); #if defined(STARBOARD) -EXTERN(SbFile *) read_stdin(void); -EXTERN(SbFile *) write_stdout(void); +EXTERN(FileStruct *) read_stdin(void); +EXTERN(FileStruct *) write_stdout(void); #else EXTERN(FILE *) read_stdin(void); EXTERN(FILE *) write_stdout(void); diff --git a/third_party/libjpeg_turbo/jmemsys.h b/third_party/libjpeg_turbo/jmemsys.h index 5cb61b26ba2a..a0329f391d89 100644 --- a/third_party/libjpeg_turbo/jmemsys.h +++ b/third_party/libjpeg_turbo/jmemsys.h @@ -143,7 +143,7 @@ typedef struct backing_store_struct { #else /* For a typical implementation with temp files, we need: */ #if defined(STARBOARD) - SbFile *temp_file; /* stdio reference to temp file */ + FileStruct *temp_file; /* stdio reference to temp file */ #else FILE *temp_file; /* stdio reference to temp file */ #endif diff --git a/third_party/libjpeg_turbo/jpeglib.h b/third_party/libjpeg_turbo/jpeglib.h index 0a09a3e3799e..f1b9b5506457 100644 --- a/third_party/libjpeg_turbo/jpeglib.h +++ b/third_party/libjpeg_turbo/jpeglib.h @@ -37,6 +37,10 @@ #include "jmorecfg.h" /* seldom changed options */ +#if defined(STARBOARD) +#include "starboard/common/file_wrapper.h" +#endif + #ifdef __cplusplus #ifndef DONT_USE_EXTERN_C extern "C" { @@ -920,8 +924,8 @@ EXTERN(void) jpeg_destroy_decompress(j_decompress_ptr cinfo); /* Standard data source and destination managers: stdio streams. */ /* Caller is responsible for opening the file before and closing after. */ #if defined(STARBOARD) -EXTERN(void) jpeg_stdio_dest(j_compress_ptr cinfo, SbFile *outfile); -EXTERN(void) jpeg_stdio_src(j_decompress_ptr cinfo, SbFile *infile); +EXTERN(void) jpeg_stdio_dest(j_compress_ptr cinfo, FileStruct *outfile); +EXTERN(void) jpeg_stdio_src(j_decompress_ptr cinfo, FileStruct *infile); #else EXTERN(void) jpeg_stdio_dest(j_compress_ptr cinfo, FILE *outfile); EXTERN(void) jpeg_stdio_src(j_decompress_ptr cinfo, FILE *infile);