Skip to content

Commit

Permalink
Revert "Deprecate SbStringFormat functions" (youtube#2083)
Browse files Browse the repository at this point in the history
Reverts youtube#1999 as it broke XB1 and PS5

b/316211534
  • Loading branch information
isarkis authored Dec 13, 2023
1 parent 22900cc commit bb7ec36
Show file tree
Hide file tree
Showing 52 changed files with 123 additions and 124 deletions.
15 changes: 11 additions & 4 deletions base/strings/string_util_starboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
#define BASE_STRING_UTIL_STARBOARD_H_

#include <stdarg.h>
#if SB_API_VERSION >= 16
#include <stdio.h>
#endif

#include "base/logging.h"
#include "base/strings/string_util.h"
Expand All @@ -28,9 +25,13 @@

namespace base {

#if defined(vsnprintf)
#undef vsnprintf
#endif

inline int vsnprintf(char* buffer, size_t size,
const char* format, va_list arguments) {
return ::vsnprintf(buffer, size, format, arguments);
return SbStringFormat(buffer, size, format, arguments);
}

inline int strncmp16(const char16* s1, const char16* s2, size_t count) {
Expand All @@ -41,6 +42,12 @@ inline int strncmp16(const char16* s1, const char16* s2, size_t count) {
#endif
}

inline int vswprintf(wchar_t* buffer, size_t size,
const wchar_t* format, va_list arguments) {
DCHECK(base::IsWprintfFormatPortable(format));
return SbStringFormatWide(buffer, size, format, arguments);
}

} // namespace base

#endif // BASE_STRING_UTIL_STARBOARD_H_
2 changes: 1 addition & 1 deletion base/strings/stringprintf_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ TEST(StringPrintfTest, Grow) {
const int kRefSize = 320000;
char* ref = new char[kRefSize];
#if defined(STARBOARD)
snprintf(ref, kRefSize, fmt, src, src, src, src, src, src, src);
SbStringFormatF(ref, kRefSize, fmt, src, src, src, src, src, src, src);
#else
#if defined(OS_WIN)
sprintf_s(ref, kRefSize, fmt, src, src, src, src, src, src, src);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const char* GetAndLeakThreadName() {
#endif // defined(OS_LINUX) || defined(OS_ANDROID)

// Use tid if we don't have a thread name.
snprintf(name, sizeof(name), "%lu",
SbStringFormatF(name, sizeof(name), "%lu",
static_cast<unsigned long>(PlatformThread::CurrentId()));
return strdup(name);
}
Expand Down
2 changes: 1 addition & 1 deletion cobalt/browser/memory_settings/pretty_print.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ std::string ToMegabyteString(int64_t bytes, int decimal_places) {
ss_fmt << "%." << decimal_places << "f MB";

char buff[128];
snprintf(buff, sizeof(buff), ss_fmt.str().c_str(), megabytes);
SbStringFormatF(buff, sizeof(buff), ss_fmt.str().c_str(), megabytes);
// Use 16
return std::string(buff);
}
Expand Down
2 changes: 1 addition & 1 deletion net/cert/internal/trust_store_in_memory_starboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ scoped_refptr<ParsedCertificate> TrustStoreInMemoryStarboard::TryLoadCert(
const base::StringPiece& cert_name) const {
auto hash = CertNameHash(cert_name.data(), cert_name.length());
char cert_file_name[256];
snprintf(cert_file_name, 256, "%08lx.%d", hash, 0);
SbStringFormatF(cert_file_name, 256, "%08lx.%d", hash, 0);

if (trusted_cert_names_on_disk_.find(cert_file_name) ==
trusted_cert_names_on_disk_.end()) {
Expand Down
6 changes: 0 additions & 6 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ since the version previous to it.

## Version 16

### Deprecated SbStringFormat APIs and migrated to POSIX memory APIs
The StringFormat management APIs `SbStringFormat`, `SbStringFormatF`,
`SbStringFormatWide`, `SbStringFormatUnsifeF` are deprecated and the
standard APIs `vsnprintf`, `vfnprintf`, `vswprintf`, `snprintf`
from `<stdlib.h>` should be used instead.

### Deprecated SbMemory allocation APIs and migrated to POSIX memory APIs
The memory management APIs `SbMemoryAllocate`, `SbMemoryReallocate`,
`SbMemoryCalloc`, `SbMemoryAllocateAligned`, `SbMemoryDeallocate`,
Expand Down
4 changes: 2 additions & 2 deletions starboard/android/shared/system_get_property.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ bool CopyAndroidPlatformName(char* out_value, int value_length) {
}

char result_string[kStringBufferSize];
snprintf(result_string, kStringBufferSize, kPlatformNameFormat,
version_string_buffer);
SbStringFormatF(result_string, kStringBufferSize, kPlatformNameFormat,
version_string_buffer);

return CopyStringAndTestIfSuccess(out_value, value_length, result_string);
}
Expand Down
10 changes: 10 additions & 0 deletions starboard/client_porting/poem/stdio_poem.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@
#include "starboard/memory.h"
#include "starboard/string.h"

// the following functions can have variable number of arguments
// and, out of compatibility concerns, we chose to not use
// __VA_ARGS__ functionality.
#undef vsnprintf
#define vsnprintf SbStringFormat
#undef snprintf
#define snprintf SbStringFormatF
#undef sprintf
#define sprintf SbStringFormatUnsafeF

#endif // POEM_NO_EMULATION

#endif // STARBOARD
Expand Down
3 changes: 3 additions & 0 deletions starboard/client_porting/poem/wchar_poem.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@

#include "starboard/string.h"

#undef vswprintf
#define vswprintf SbStringFormatWide

#endif // POEM_NO_EMULATION

#endif // STARBOARD
Expand Down
7 changes: 2 additions & 5 deletions starboard/common/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@
#define STARBOARD_COMMON_STRING_H_

#include <stdarg.h>
#if SB_API_VERSION >= 16
#include <stdio.h>
#endif
#include <cstring>
#include <string>
#include <vector>
Expand All @@ -38,7 +35,7 @@ SB_C_INLINE std::string FormatString(const char* format, ...)
SB_C_INLINE std::string FormatString(const char* format, ...) {
va_list arguments;
va_start(arguments, format);
int expected_size = vsnprintf(NULL, 0, format, arguments);
int expected_size = ::SbStringFormat(NULL, 0, format, arguments);
va_end(arguments);

std::string result;
Expand All @@ -48,7 +45,7 @@ SB_C_INLINE std::string FormatString(const char* format, ...) {

std::vector<char> buffer(expected_size + 1);
va_start(arguments, format);
vsnprintf(buffer.data(), buffer.size(), format, arguments);
::SbStringFormat(buffer.data(), buffer.size(), format, arguments);
va_end(arguments);
return std::string(buffer.data(), expected_size);
}
Expand Down
8 changes: 0 additions & 8 deletions starboard/doc/c99.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,3 @@ deprecated and eventually removed.
* wmemmove
* wmemset
* wcsncmp
* snprintf
* sprintf
* vfwprintf
* vsnprintf
* vswprintf
* ferror
* fputwc
* fwide
16 changes: 2 additions & 14 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,10 @@ ExportedSymbols::ExportedSymbols() {
REGISTER_SYMBOL(SbStringCompareNoCase);
REGISTER_SYMBOL(SbStringCompareNoCaseN);
REGISTER_SYMBOL(SbStringDuplicate);
#endif // SB_API_VERSION < 16
REGISTER_SYMBOL(SbStringFormat);
REGISTER_SYMBOL(SbStringFormatWide);
#if SB_API_VERSION < 16
REGISTER_SYMBOL(SbStringScan);
#endif // SB_API_VERSION < 16
REGISTER_SYMBOL(SbSystemBreakIntoDebugger);
Expand Down Expand Up @@ -404,20 +406,6 @@ ExportedSymbols::ExportedSymbols() {
REGISTER_SYMBOL(calloc);
REGISTER_SYMBOL(posix_memalign);
REGISTER_SYMBOL(free);
REGISTER_SYMBOL(sprintf);
REGISTER_SYMBOL(snprintf);
REGISTER_SYMBOL(vfwprintf);
REGISTER_SYMBOL(vsnprintf);
#if defined(_MSC_VER)
// MSVC provides a template with the same name.
// The cast helps the compiler to pick the correct C function pointer to be
// used.
REGISTER_SYMBOL(
static_cast<int (*)(wchar_t* buffer, size_t count, const wchar_t* format,
va_list argptr)>(vswprintf));
#else
REGISTER_SYMBOL(vswprintf);
#endif // defined(_MSC_VER)
REGISTER_SYMBOL(vsscanf);
#endif // SB_API_VERSION >= 16

Expand Down
11 changes: 6 additions & 5 deletions starboard/linux/shared/system_get_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ bool GetCacheDirectory(char* out_path, int path_size) {
kMaxPathSize)) {
return false;
}
int result = snprintf(out_path, path_size, "%s/.cache", home_path.data());
int result =
SbStringFormatF(out_path, path_size, "%s/.cache", home_path.data());
if (result < 0 || result >= path_size) {
out_path[0] = '\0';
return false;
Expand All @@ -56,8 +57,8 @@ bool GetStorageDirectory(char* out_path, int path_size) {
kMaxPathSize)) {
return false;
}
int result =
snprintf(out_path, path_size, "%s/.cobalt_storage", home_path.data());
int result = SbStringFormatF(out_path, path_size, "%s/.cobalt_storage",
home_path.data());
if (result < 0 || result >= path_size) {
out_path[0] = '\0';
return false;
Expand Down Expand Up @@ -158,8 +159,8 @@ bool GetTemporaryDirectory(char* out_path, int path_size) {
return false;
}

int result = snprintf(out_path, path_size, "/tmp/%s-%d", binary_name.data(),
static_cast<int>(getpid()));
int result = SbStringFormatF(out_path, path_size, "/tmp/%s-%d",
binary_name.data(), static_cast<int>(getpid()));
if (result < 0 || result >= path_size) {
out_path[0] = '\0';
return false;
Expand Down
25 changes: 13 additions & 12 deletions starboard/loader_app/installation_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,18 @@ std::string InstallationManager::DumpInstallationSlots() {
out << "size=";
const int kBufSize = 50;
char buf_num[kBufSize];
snprintf(buf_num, kBufSize, "%d", installation_store_.installations_size());
SbStringFormatF(buf_num, kBufSize, "%d",
installation_store_.installations_size());
out << buf_num;

out << " roll_forward_to_installation=";
snprintf(buf_num, kBufSize, "%d",
installation_store_.roll_forward_to_installation());
SbStringFormatF(buf_num, kBufSize, "%d",
installation_store_.roll_forward_to_installation());
out << buf_num;
out << ";";
for (int i = 0; i < installation_store_.installations_size(); i++) {
out << " installation_";
snprintf(buf_num, kBufSize, "%d", i);
SbStringFormatF(buf_num, kBufSize, "%d", i);
out << buf_num;
out << " is_successful=";
if (installation_store_.installations(i).is_successful()) {
Expand All @@ -221,13 +222,13 @@ std::string InstallationManager::DumpInstallationSlots() {
}

out << " num_tries_left=";
snprintf(buf_num, kBufSize, "%d",
installation_store_.installations(i).num_tries_left());
SbStringFormatF(buf_num, kBufSize, "%d",
installation_store_.installations(i).num_tries_left());
out << buf_num;

out << " priority=";
snprintf(buf_num, kBufSize, "%d",
installation_store_.installations(i).priority());
SbStringFormatF(buf_num, kBufSize, "%d",
installation_store_.installations(i).priority());
out << buf_num;
out << ";";
}
Expand Down Expand Up @@ -685,11 +686,11 @@ bool InstallationManager::GetInstallationPathInternal(int installation_index,
// SLOT_0 is placed in |kSbSystemPathContentDirectory|, under the subdirectory
// 'app/cobalt'.
if (installation_index == 0) {
snprintf(path, path_length, "%s%s%s%s%s", content_dir_.c_str(),
kSbFileSepString, "app", kSbFileSepString, "cobalt");
SbStringFormatF(path, path_length, "%s%s%s%s%s", content_dir_.c_str(),
kSbFileSepString, "app", kSbFileSepString, "cobalt");
} else {
snprintf(path, path_length, "%s%s%s%d", storage_dir_.c_str(),
kSbFileSepString, "installation_", installation_index);
SbStringFormatF(path, path_length, "%s%s%s%d", storage_dir_.c_str(),
kSbFileSepString, "installation_", installation_index);
}

return true;
Expand Down
18 changes: 10 additions & 8 deletions starboard/loader_app/slot_management.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,14 @@ void* LoadSlotManagedLibrary(const std::string& app_key,

// installation_n/lib/libcobalt.so
std::vector<char> compressed_lib_path(kSbFileMaxPath);
snprintf(compressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString, kCobaltLibraryPath,
kSbFileSepString, kCompressedCobaltLibraryName);
SbStringFormatF(compressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltLibraryPath, kSbFileSepString,
kCompressedCobaltLibraryName);
std::vector<char> uncompressed_lib_path(kSbFileMaxPath);
snprintf(uncompressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString, kCobaltLibraryPath,
kSbFileSepString, kCobaltLibraryName);
SbStringFormatF(uncompressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltLibraryPath, kSbFileSepString, kCobaltLibraryName);

std::string lib_path;
bool use_compression;
Expand Down Expand Up @@ -257,8 +258,9 @@ void* LoadSlotManagedLibrary(const std::string& app_key,
if (alternative_content_path.empty()) {
// installation_n/content
std::vector<char> content_path(kSbFileMaxPath);
snprintf(content_path.data(), kSbFileMaxPath, "%s%s%s",
installation_path.data(), kSbFileSepString, kCobaltContentPath);
SbStringFormatF(content_path.data(), kSbFileMaxPath, "%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltContentPath);
content = content_path.data();
} else {
content = alternative_content_path.c_str();
Expand Down
2 changes: 0 additions & 2 deletions starboard/nplb/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,6 @@ target(gtest_target_type, "nplb") {
"string_compare_no_case_n_test.cc",
"string_compare_no_case_test.cc",
"string_duplicate_test.cc",

# TODO: b/307941391: test "SbStringFormatWideTest" is deprecated in SB16
"string_format_test.cc",
"string_format_wide_test.cc",
"string_scan_test.cc",
Expand Down
3 changes: 0 additions & 3 deletions starboard/nplb/string_format_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

// Here we are not trying to do anything fancy, just to really sanity check that
// this is hooked up to something.
#if SB_API_VERSION < 16

#include "starboard/common/string.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -46,5 +45,3 @@ TEST(SbStringFormatTest, SunnyDay) {
} // namespace
} // namespace nplb
} // namespace starboard

#endif // SB_API_VERSION < 16
3 changes: 0 additions & 3 deletions starboard/nplb/string_format_wide_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

// Here we are not trying to do anything fancy, just to really sanity check that
// this is hooked up to something.
#if SB_API_VERSION < 16

#include "starboard/common/string.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -49,5 +48,3 @@ TEST(SbStringFormatWideTest, SunnyDay) {
} // namespace
} // namespace nplb
} // namespace starboard

#endif // SB_API_VERSION < 16
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ const int kMaxVersionedLibraryNameLength = 32;

std::string GetVersionedLibraryName(const char* name, const int version) {
char s[kMaxVersionedLibraryNameLength];
snprintf(s, kMaxVersionedLibraryNameLength, "%s.%d", name, version);
SbStringFormatF(s, kMaxVersionedLibraryNameLength, "%s.%d", name, version);
return std::string(s);
}

Expand Down
2 changes: 0 additions & 2 deletions starboard/shared/posix/string_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
#include <stdarg.h>
#include <stdio.h>

#if SB_API_VERSION < 16
int SbStringFormat(char* out_buffer,
size_t buffer_size,
const char* format,
va_list arguments) {
return vsnprintf(out_buffer, buffer_size, format, arguments);
}
#endif
2 changes: 0 additions & 2 deletions starboard/shared/posix/string_format_wide.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@
#include <stdio.h>
#include <wchar.h>

#if SB_API_VERSION < 16
int SbStringFormatWide(wchar_t* out_buffer,
size_t buffer_size,
const wchar_t* format,
va_list arguments) {
return vswprintf(out_buffer, buffer_size, format, arguments);
}
#endif
2 changes: 1 addition & 1 deletion starboard/shared/starboard/link_receiver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ void LinkReceiver::Impl::Run() {
}

char port_string[32] = {0};
snprintf(port_string, SB_ARRAY_SIZE(port_string), "%d", actual_port_);
SbStringFormatF(port_string, SB_ARRAY_SIZE(port_string), "%d", actual_port_);
CreateTemporaryFile("link_receiver_port", port_string, strlen(port_string));

if (!AddForAccept(listen_socket_.get())) {
Expand Down
Loading

0 comments on commit bb7ec36

Please sign in to comment.