Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate SbStringFormat functions #1999

Merged
merged 3 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions base/strings/string_util_starboard.h
maxz-lab marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#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 @@ -25,13 +28,9 @@

namespace base {

#if defined(vsnprintf)
#undef vsnprintf
#endif

inline int vsnprintf(char* buffer, size_t size,
const char* format, va_list arguments) {
return SbStringFormat(buffer, size, format, arguments);
return ::vsnprintf(buffer, size, format, arguments);
maxz-lab marked this conversation as resolved.
Show resolved Hide resolved
}

inline int strncmp16(const char16* s1, const char16* s2, size_t count) {
Expand All @@ -42,12 +41,6 @@ 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)
SbStringFormatF(ref, kRefSize, fmt, src, src, src, src, src, src, src);
snprintf(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.
SbStringFormatF(name, sizeof(name), "%lu",
snprintf(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];
SbStringFormatF(buff, sizeof(buff), ss_fmt.str().c_str(), megabytes);
snprintf(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];
SbStringFormatF(cert_file_name, 256, "%08lx.%d", hash, 0);
snprintf(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: 6 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ 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];
SbStringFormatF(result_string, kStringBufferSize, kPlatformNameFormat,
version_string_buffer);
snprintf(result_string, kStringBufferSize, kPlatformNameFormat,
version_string_buffer);

return CopyStringAndTestIfSuccess(out_value, value_length, result_string);
}
Expand Down
10 changes: 0 additions & 10 deletions starboard/client_porting/poem/stdio_poem.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@
#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: 0 additions & 3 deletions starboard/client_porting/poem/wchar_poem.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

#include "starboard/string.h"

#undef vswprintf
#define vswprintf SbStringFormatWide

#endif // POEM_NO_EMULATION

#endif // STARBOARD
Expand Down
7 changes: 5 additions & 2 deletions starboard/common/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
#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 @@ -35,7 +38,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 = ::SbStringFormat(NULL, 0, format, arguments);
int expected_size = vsnprintf(NULL, 0, format, arguments);
va_end(arguments);

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

std::vector<char> buffer(expected_size + 1);
va_start(arguments, format);
::SbStringFormat(buffer.data(), buffer.size(), format, arguments);
vsnprintf(buffer.data(), buffer.size(), format, arguments);
va_end(arguments);
return std::string(buffer.data(), expected_size);
}
Expand Down
8 changes: 8 additions & 0 deletions starboard/doc/c99.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,11 @@ deprecated and eventually removed.
* wmemmove
* wmemset
* wcsncmp
* snprintf
* sprintf
* vfwprintf
* vsnprintf
* vswprintf
* ferror
* fputwc
* fwide
16 changes: 14 additions & 2 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,8 @@ 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 @@ -406,6 +404,20 @@ 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: 5 additions & 6 deletions starboard/linux/shared/system_get_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ bool GetCacheDirectory(char* out_path, int path_size) {
kMaxPathSize)) {
return false;
}
int result =
SbStringFormatF(out_path, path_size, "%s/.cache", home_path.data());
int result = snprintf(out_path, path_size, "%s/.cache", home_path.data());
if (result < 0 || result >= path_size) {
out_path[0] = '\0';
return false;
Expand All @@ -57,8 +56,8 @@ bool GetStorageDirectory(char* out_path, int path_size) {
kMaxPathSize)) {
return false;
}
int result = SbStringFormatF(out_path, path_size, "%s/.cobalt_storage",
home_path.data());
int result =
snprintf(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 @@ -159,8 +158,8 @@ bool GetTemporaryDirectory(char* out_path, int path_size) {
return false;
}

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

out << " roll_forward_to_installation=";
SbStringFormatF(buf_num, kBufSize, "%d",
installation_store_.roll_forward_to_installation());
snprintf(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_";
SbStringFormatF(buf_num, kBufSize, "%d", i);
snprintf(buf_num, kBufSize, "%d", i);
out << buf_num;
out << " is_successful=";
if (installation_store_.installations(i).is_successful()) {
Expand All @@ -222,13 +221,13 @@ std::string InstallationManager::DumpInstallationSlots() {
}

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

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

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

// installation_n/lib/libcobalt.so
std::vector<char> compressed_lib_path(kSbFileMaxPath);
SbStringFormatF(compressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltLibraryPath, kSbFileSepString,
kCompressedCobaltLibraryName);
snprintf(compressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString, kCobaltLibraryPath,
kSbFileSepString, kCompressedCobaltLibraryName);
std::vector<char> uncompressed_lib_path(kSbFileMaxPath);
SbStringFormatF(uncompressed_lib_path.data(), kSbFileMaxPath, "%s%s%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltLibraryPath, kSbFileSepString, kCobaltLibraryName);
snprintf(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 @@ -258,9 +257,8 @@ void* LoadSlotManagedLibrary(const std::string& app_key,
if (alternative_content_path.empty()) {
// installation_n/content
std::vector<char> content_path(kSbFileMaxPath);
SbStringFormatF(content_path.data(), kSbFileMaxPath, "%s%s%s",
installation_path.data(), kSbFileSepString,
kCobaltContentPath);
snprintf(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: 2 additions & 0 deletions starboard/nplb/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ 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: 3 additions & 0 deletions starboard/nplb/string_format_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

// 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 @@ -45,3 +46,5 @@ TEST(SbStringFormatTest, SunnyDay) {
} // namespace
} // namespace nplb
} // namespace starboard

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

// 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 @@ -48,3 +49,5 @@ 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];
SbStringFormatF(s, kMaxVersionedLibraryNameLength, "%s.%d", name, version);
snprintf(s, kMaxVersionedLibraryNameLength, "%s.%d", name, version);
return std::string(s);
}

Expand Down
2 changes: 2 additions & 0 deletions starboard/shared/posix/string_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
#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: 2 additions & 0 deletions starboard/shared/posix/string_format_wide.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
#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};
SbStringFormatF(port_string, SB_ARRAY_SIZE(port_string), "%d", actual_port_);
snprintf(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
Loading