Skip to content

Commit

Permalink
WIP: Deprecate SbStringFormat functions
Browse files Browse the repository at this point in the history
Replace SbStringFormat*() calls with POSIX vsnprintf().
  SbStringFormat        -> vsnprintf
  SbStringFormatF       -> snprintf
  SbStringFormatWide    -> vswprintf
  SbStringFormatUnsafeF -> sprintf

Backward compatibility is protected by "SB_API_VERSION < 16".

Tested on Linux, Android, Evergreen, Win32

b/307941391

Change-Id: I437ff3d7bf735b46767867ea844aeffb28e58b4d
  • Loading branch information
maxz-lab committed Nov 17, 2023
1 parent 0ee9237 commit 44cc6fc
Show file tree
Hide file tree
Showing 42 changed files with 88 additions and 107 deletions.
3 changes: 2 additions & 1 deletion base/strings/string_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <ctype.h>
#include <stdarg.h> // va_list
#include <stdio.h>

#include <initializer_list>
#include <string>
Expand Down Expand Up @@ -47,7 +48,7 @@ inline int snprintf(char* buffer,
...) {
va_list arguments;
va_start(arguments, format);
int result = vsnprintf(buffer, size, format, arguments);
int result = ::vsnprintf(buffer, size, format, arguments);
va_end(arguments);
return result;
}
Expand Down
15 changes: 0 additions & 15 deletions base/strings/string_util_starboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,6 @@ inline int strncasecmp(const char* string1, const char* string2, size_t count) {
return SbStringCompareNoCaseN(string1, string2, count);
}

#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);
}

inline int strncmp16(const char16* s1, const char16* s2, size_t count) {
#if defined(WCHAR_T_IS_UTF16)
return wcsncmp(s1, s2, count);
Expand All @@ -50,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.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ inline int vsnprintfT(char* buffer,
size_t buf_size,
const char* format,
va_list argptr) {
return base::vsnprintf(buffer, buf_size, format, argptr);
return ::vsnprintf(buffer, buf_size, format, argptr);
}

#if defined(OS_WIN)
Expand Down
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
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
6 changes: 0 additions & 6 deletions starboard/client_porting/poem/stdio_poem.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@
// 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
#undef vsscanf
#define vsscanf SbStringScan
#undef sscanf
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
5 changes: 3 additions & 2 deletions starboard/common/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define STARBOARD_COMMON_STRING_H_

#include <stdarg.h>
#include <stdio.h>
#include <cstring>
#include <string>
#include <vector>
Expand All @@ -35,7 +36,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 +46,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
3 changes: 1 addition & 2 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,9 @@ ExportedSymbols::ExportedSymbols() {

#if SB_API_VERSION < 16
REGISTER_SYMBOL(SbStringDuplicate);
#endif // #if SB_API_VERSION < 16

REGISTER_SYMBOL(SbStringFormat);
REGISTER_SYMBOL(SbStringFormatWide);
#endif
REGISTER_SYMBOL(SbStringScan);
REGISTER_SYMBOL(SbSystemBreakIntoDebugger);
REGISTER_SYMBOL(SbSystemClearLastError);
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
2 changes: 1 addition & 1 deletion starboard/nplb/string_format_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace {
int Format(char* out_buffer, size_t buffer_size, const char* format, ...) {
va_list arguments;
va_start(arguments, format);
int result = SbStringFormat(out_buffer, buffer_size, format, arguments);
int result = vsnprintf(out_buffer, buffer_size, format, arguments);
va_end(arguments);
return result;
}
Expand Down
2 changes: 2 additions & 0 deletions starboard/nplb/string_format_wide_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace starboard {
namespace nplb {
namespace {

#if SB_API_VERSION <= 15
int Format(wchar_t* out_buffer,
size_t buffer_size,
const wchar_t* format,
Expand All @@ -44,6 +45,7 @@ TEST(SbStringFormatWideTest, SunnyDay) {
EXPECT_EQ(kExpected[i], destination[i]);
}
}
#endif

} // namespace
} // namespace nplb
Expand Down
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
3 changes: 2 additions & 1 deletion starboard/shared/starboard/log_raw_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
#include "starboard/common/log.h"

#include <stdarg.h>
#include <stdio.h>

#include "starboard/common/string.h"

void SbLogRawFormat(const char* format, va_list arguments) {
char message[128];
SbStringFormat(message, sizeof(message), format, arguments);
vsnprintf(message, sizeof(message), format, arguments);
SbLogRaw(message);
}
2 changes: 2 additions & 0 deletions starboard/shared/stub/string_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@

#include "starboard/common/string.h"

#if SB_API_VERSION <= 15
int SbStringFormat(char* out_buffer,
size_t buffer_size,
const char* format,
va_list arguments) {
return 0;
}
#endif
2 changes: 2 additions & 0 deletions starboard/shared/stub/string_format_wide.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@

#include "starboard/common/string.h"

#if SB_API_VERSION <= 15
int SbStringFormatWide(wchar_t* out_buffer,
size_t buffer_size,
const wchar_t* format,
va_list arguments) {
return 0;
}
#endif
Loading

0 comments on commit 44cc6fc

Please sign in to comment.