Skip to content

Commit

Permalink
Revert "Revert "Deprecate SbDirectoryCreate"" (youtube#2366)
Browse files Browse the repository at this point in the history
Reverts youtube#2362

b/302730696
  • Loading branch information
madhurajayaraman authored Feb 2, 2024
1 parent eaeaf5e commit 4819ff6
Show file tree
Hide file tree
Showing 35 changed files with 300 additions and 33 deletions.
7 changes: 4 additions & 3 deletions base/files/file_util_starboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <stack>
#include <string>
#include <sys/stat.h>

#include "base/base_paths.h"
#include "base/files/file_enumerator.h"
Expand Down Expand Up @@ -317,7 +318,8 @@ bool CreateDirectoryAndGetError(const FilePath &full_path, File::Error* error) {

// Fast-path: can the full path be resolved from the full path?
if (DirectoryExists(full_path) ||
SbDirectoryCreate(full_path.value().c_str())) {
mkdir(full_path.value().c_str(), 0700) == 0 ||
SbDirectoryCanOpen(full_path.value().c_str())) {
return true;
}

Expand All @@ -341,8 +343,7 @@ bool CreateDirectoryAndGetError(const FilePath &full_path, File::Error* error) {
if (DirectoryExists(*i)) {
continue;
}

if (!SbDirectoryCreate(i->value().c_str())) {
if (mkdir(i->value().c_str(), 0700) != 0 && !SbDirectoryCanOpen(i->value().c_str())) {
if (error)
*error = File::OSErrorToFileError(SbSystemGetLastError());
return false;
Expand Down
3 changes: 2 additions & 1 deletion cobalt/browser/splash_screen_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "starboard/common/string.h"
#include "starboard/configuration_constants.h"
#include "starboard/directory.h"
#include "sys/stat.h"

namespace cobalt {
namespace browser {
Expand All @@ -46,7 +47,7 @@ bool CreateDirsForKey(const std::string& key) {
starboard::strlcat(path.data(),
key.substr(prev_found, found - prev_found).c_str(),
kSbFileMaxPath);
if (!SbDirectoryCreate(path.data())) {
if (!SbDirectoryCanOpen(path.data()) && mkdir(path.data(), 0700) != 0) {
return false;
}
prev_found = found;
Expand Down
7 changes: 5 additions & 2 deletions cobalt/cache/memory_capped_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include "cobalt/cache/memory_capped_directory.h"

#include <sys/stat.h>

#include <algorithm>
#include <string>

Expand Down Expand Up @@ -54,7 +56,8 @@ bool MemoryCappedDirectory::FileInfo::OldestFirst::operator()(
// static
std::unique_ptr<MemoryCappedDirectory> MemoryCappedDirectory::Create(
const base::FilePath& directory_path, uint32_t max_size) {
if (!SbDirectoryCreate(directory_path.value().c_str())) {
if (!SbDirectoryCanOpen(directory_path.value().c_str()) &&
mkdir(directory_path.value().c_str(), 0700) != 0) {
return nullptr;
}
auto memory_capped_directory = std::unique_ptr<MemoryCappedDirectory>(
Expand Down Expand Up @@ -118,7 +121,7 @@ void MemoryCappedDirectory::DeleteAll() {
// Recursively delete the contents of the directory_path_.
base::DeleteFile(directory_path_, true);
// Re-create the directory_path_ which will now be empty.
SbDirectoryCreate(directory_path_.value().c_str());
mkdir(directory_path_.value().c_str(), 0700);
file_info_heap_.clear();
file_sizes_.clear();
file_keys_with_metadata_.clear();
Expand Down
29 changes: 29 additions & 0 deletions cobalt/updater/utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "cobalt/updater/utils.h"

#include <vector>
#include <sys/stat.h>

#include "base/files/file_path.h"
#include "base/strings/strcat.h"
Expand Down Expand Up @@ -133,7 +134,11 @@ class UtilsTest : public testing::Test {
const std::string& installation_path) {
std::string lib_path = base::StrCat(
{installation_path, kSbFileSepString, kEvergreenLibDirname});
#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(lib_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(lib_path.c_str()) || mkdir(lib_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16

lib_path = base::StrCat({lib_path, kSbFileSepString, name});
SbFile sb_file = SbFileOpen(
Expand All @@ -155,7 +160,11 @@ class UtilsTest : public testing::Test {
TEST_F(UtilsTest, ReadEvergreenVersionReturnsVersionForValidManifest) {
std::string installation_path = base::StrCat(
{temp_dir_path_.data(), kSbFileSepString, "some_installation_path"});
#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
char manifest_content[] = R"json(
{
"manifest_version": 2,
Expand All @@ -177,7 +186,11 @@ TEST_F(UtilsTest,
ReadEvergreenVersionReturnsInvalidVersionForVersionlessManifest) {
std::string installation_path = base::StrCat(
{temp_dir_path_.data(), kSbFileSepString, "some_installation_path"});
#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
char versionless_manifest_content[] = R"json(
{
"manifest_version": 2,
Expand Down Expand Up @@ -222,7 +235,11 @@ TEST_F(UtilsTest, ReturnsEvergreenVersionFromCurrentManagedInstallation) {
std::function<const void*(const char*)> stub_get_extension_fn =
[](const char* name) { return &kStubInstallationManagerApi; };

#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
char manifest_content[] = R"json(
{
"manifest_version": 2,
Expand Down Expand Up @@ -261,7 +278,11 @@ TEST_F(UtilsTest,
std::function<const void*(const char*)> stub_get_extension_fn =
[](const char* name) { return &kStubInstallationManagerApi; };

#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
// No manifest is created in the installation directory.

std::string version = GetCurrentEvergreenVersion(stub_get_extension_fn);
Expand Down Expand Up @@ -421,7 +442,11 @@ TEST_F(UtilsTest,
std::function<const void*(const char*)> stub_get_extension_fn =
[](const char* name) { return &kStubInstallationManagerApi; };

#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
char manifest_content[] = R"json(
{
"manifest_version": 2,
Expand Down Expand Up @@ -464,7 +489,11 @@ TEST_F(UtilsTest,
std::function<const void*(const char*)> stub_get_extension_fn =
[](const char* name) { return &kStubInstallationManagerApi; };

#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(installation_path.c_str()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(installation_path.c_str()) || mkdir(installation_path.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
CreateEmptyLibrary("libcobalt.unexpected", installation_path);

EvergreenLibraryMetadata metadata =
Expand Down
4 changes: 4 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ Config OBUs as side data.
### Deprecated SbStringScan and SbStringScanF
The APIs defined in `starboard/string.h` are deprecated and the standard API `vsscanf` and `sscanf` are used instead.

### Decrecated SbDirectory APIs and migrated to POSIX directory APIs
The directory API `SbDirectoryCreate` is deprecated and the standard APIs
`mkdir` from `<sys/stat.h>` should be used instead.

## Version 15

### SbMemoryReporter is no longer used
Expand Down
4 changes: 3 additions & 1 deletion starboard/android/shared/android_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <sys/stat.h>

#include "game-activity/GameActivity.h"
#include "starboard/android/shared/application_android.h"
Expand Down Expand Up @@ -181,7 +182,8 @@ std::string ExtractCertificatesToFileSystem() {

std::string file_system_path(file_system_path_buffer.data());
file_system_path.append(std::string(kSbFileSepString) + "certs");
if (!SbDirectoryCreate(file_system_path.c_str())) {
if (mkdir(file_system_path.c_str(), 0700) != 0 ||
!SbDirectoryCanOpen(file_system_path.c_str())) {
SB_LOG(WARNING) << "Failed to create new dir for CA certificates";
return "";
}
Expand Down
8 changes: 4 additions & 4 deletions starboard/android/shared/system_get_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
if (starboard::strlcat(path, "/storage", kPathSize) >= kPathSize) {
return false;
}
SbDirectoryCreate(path);
mkdir(path, 0700);
break;
}
case kSbSystemPathCacheDirectory: {
Expand All @@ -110,7 +110,7 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
return false;
}

SbDirectoryCreate(path);
mkdir(path, 0700);
break;
}

Expand All @@ -122,7 +122,7 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
return false;
}

SbDirectoryCreate(path);
mkdir(path, 0700);
break;
}

Expand All @@ -131,7 +131,7 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
return false;
}

SbDirectoryCreate(path);
mkdir(path, 0700);
break;
}

Expand Down
3 changes: 2 additions & 1 deletion starboard/directory.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@ SB_EXPORT bool SbDirectoryGetNext(SbDirectory directory,
// |path|: The path to be checked.
SB_EXPORT bool SbDirectoryCanOpen(const char* path);

#if SB_API_VERSION < 16
// Creates the directory |path|, assuming the parent directory already exists.
// This function returns |true| if the directory now exists (even if it existed
// before) and returns |false| if the directory does not exist.
//
// |path|: The path to be created.
SB_EXPORT bool SbDirectoryCreate(const char* path);

#endif // SB_API_VERSION < 16
#ifdef __cplusplus
} // extern "C"
#endif
Expand Down
5 changes: 5 additions & 0 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <stdlib.h>
#include <sys/mman.h>
#include <sys/stat.h>

#include "starboard/accessibility.h"
#include "starboard/audio_sink.h"
Expand Down Expand Up @@ -46,6 +47,7 @@
#if SB_API_VERSION >= 16
#include "starboard/shared/modular/posix_mmap_wrappers.h"
#include "starboard/shared/modular/posix_time_wrappers.h"

#endif // SB_API_VERSION >= 16
#include "starboard/socket.h"
#include "starboard/socket_waiter.h"
Expand Down Expand Up @@ -136,7 +138,9 @@ ExportedSymbols::ExportedSymbols() {
REGISTER_SYMBOL(SbDecodeTargetRelease);
REGISTER_SYMBOL(SbDirectoryCanOpen);
REGISTER_SYMBOL(SbDirectoryClose);
#if SB_API_VERSION < 16
REGISTER_SYMBOL(SbDirectoryCreate);
#endif // SB_API_VERSION < 16
REGISTER_SYMBOL(SbDirectoryGetNext);
REGISTER_SYMBOL(SbDirectoryOpen);
REGISTER_SYMBOL(SbDrmCloseSession);
Expand Down Expand Up @@ -419,6 +423,7 @@ ExportedSymbols::ExportedSymbols() {
REGISTER_SYMBOL(mprotect);
REGISTER_SYMBOL(munmap);
REGISTER_SYMBOL(msync);
REGISTER_SYMBOL(mkdir);

// Custom mapped POSIX APIs to compatibility wrappers.
// These will rely on Starboard-side implementations that properly translate
Expand Down
12 changes: 7 additions & 5 deletions starboard/linux/shared/system_get_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ bool GetCacheDirectory(char* out_path, int path_size) {
out_path[0] = '\0';
return false;
}
return SbDirectoryCreate(out_path);
bool exists = SbDirectoryCanOpen(out_path);
return (mkdir(out_path, 0700) == 0 || exists);
}

// Gets the path to the storage directory, using the home directory.
Expand All @@ -62,7 +63,8 @@ bool GetStorageDirectory(char* out_path, int path_size) {
out_path[0] = '\0';
return false;
}
return SbDirectoryCreate(out_path);
bool exists = SbDirectoryCanOpen(out_path);
return (mkdir(out_path, 0700) == 0 || exists);
}

// Places up to |path_size| - 1 characters of the path to the current
Expand Down Expand Up @@ -214,7 +216,7 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
if (starboard::strlcat(path.data(), "/cobalt", kPathSize) >= kPathSize) {
return false;
}
if (!SbDirectoryCreate(path.data())) {
if (mkdir(path.data(), 0700) != 0 && !SbDirectoryCanOpen(path.data())) {
return false;
}
break;
Expand All @@ -227,14 +229,14 @@ bool SbSystemGetPath(SbSystemPathId path_id, char* out_path, int path_size) {
if (starboard::strlcat(path.data(), "/log", kPathSize) >= kPathSize) {
return false;
}
SbDirectoryCreate(path.data());
mkdir(path.data(), 0700);
break;

case kSbSystemPathTempDirectory:
if (!GetTemporaryDirectory(path.data(), kPathSize)) {
return false;
}
SbDirectoryCreate(path.data());
mkdir(path.data(), 0700);
break;

#if SB_API_VERSION < 14
Expand Down
5 changes: 4 additions & 1 deletion starboard/loader_app/app_key_files_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include "starboard/configuration_constants.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace starboard {
namespace loader_app {
namespace {
Expand All @@ -36,7 +35,11 @@ class AppKeyFilesTest : public testing::Test {
dir_ = temp_path.data();
dir_ += kSbFileSepString;
dir_ += kTestAppKeyDir;
#if SB_API_VERSION < 16
SbDirectoryCreate(dir_.c_str());
#else
mkdir(dir_.c_str(), 0700);
#endif // SB_API_VERSION < 16
}

std::string dir_;
Expand Down
10 changes: 9 additions & 1 deletion starboard/loader_app/drain_file_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "starboard/system.h"
#include "starboard/types.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace starboard {
namespace loader_app {
namespace {
Expand All @@ -46,7 +45,12 @@ class DrainFileTest : public ::testing::Test {
// Use dedicated dir for testing to avoid meddling with other files.
starboard::strlcat(temp_dir_.data(), kSbFileSepString, kSbFileMaxPath);
starboard::strlcat(temp_dir_.data(), "df", kSbFileMaxPath);
#if SB_API_VERSION < 16
ASSERT_TRUE(SbDirectoryCreate(temp_dir_.data()));
#else
ASSERT_TRUE(SbDirectoryCanOpen(temp_dir_.data()) ||
mkdir(temp_dir_.data(), 0700) == 0);
#endif // SB_API_VERSION < 16
}

void TearDown() override { DrainFileClearForApp(GetTempDir(), ""); }
Expand Down Expand Up @@ -223,7 +227,11 @@ TEST_F(DrainFileTest, SunnyDayPrepareDirectory) {
dir.append(kSbFileSepString);
dir.append("to_delete");

#if SB_API_VERSION < 16
EXPECT_TRUE(SbDirectoryCreate(dir.c_str()));
#else
EXPECT_TRUE(SbDirectoryCanOpen(dir.c_str()) || mkdir(dir.c_str(), 0700) == 0);
#endif // SB_API_VERSION < 16
EXPECT_TRUE(SbFileExists(dir.c_str()));

// Create a file with the app key in the name.
Expand Down
4 changes: 2 additions & 2 deletions starboard/loader_app/installation_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "starboard/loader_app/installation_manager.h"

#include <sys/stat.h>
#include <set>
#include <string>
#include <vector>
Expand Down Expand Up @@ -706,8 +707,7 @@ bool InstallationManager::CreateInstallationDirs() {
if (!GetInstallationPathInternal(i, path.data(), kSbFileMaxPath)) {
return false;
}

if (!SbDirectoryCreate(path.data())) {
if (!SbDirectoryCanOpen(path.data()) && mkdir(path.data(), 0700) != 0) {
return false;
}
}
Expand Down
Loading

0 comments on commit 4819ff6

Please sign in to comment.