EDK: Add mojo::util::StringPrintf() (etc.) and convert away from base's.
This is loosely based on Chromium's base::StringPrintf() (but
simplified/improved). Note that the names of "append f", etc. are also
made more consistent (ditto for "v printf", etc.).
R=vardhan@google.com
Review URL: https://codereview.chromium.org/1486923002 .
diff --git a/mojo/edk/system/channel.cc b/mojo/edk/system/channel.cc
index c26a374..69227ab 100644
--- a/mojo/edk/system/channel.cc
+++ b/mojo/edk/system/channel.cc
@@ -9,14 +9,15 @@
#include "base/bind.h"
#include "base/logging.h"
-#include "base/strings/stringprintf.h"
#include "mojo/edk/system/endpoint_relayer.h"
#include "mojo/edk/system/transport_data.h"
+#include "mojo/edk/util/string_printf.h"
using mojo::platform::ScopedPlatformHandle;
using mojo::util::MakeRefCounted;
using mojo::util::MutexLocker;
using mojo::util::RefPtr;
+using mojo::util::StringPrintf;
namespace mojo {
namespace system {
@@ -162,10 +163,10 @@
if (!SendControlMessage(MessageInTransit::Subtype::CHANNEL_REMOVE_ENDPOINT,
local_id, remote_id, 0, nullptr)) {
HandleLocalError(
- base::StringPrintf("Failed to send message to remove remote endpoint "
- "(local ID %u, remote ID %u)",
- static_cast<unsigned>(local_id.value()),
- static_cast<unsigned>(remote_id.value()))
+ StringPrintf("Failed to send message to remove remote endpoint (local "
+ "ID %u, remote ID %u)",
+ static_cast<unsigned>(local_id.value()),
+ static_cast<unsigned>(remote_id.value()))
.c_str());
}
}
@@ -309,10 +310,9 @@
OnReadMessageForChannel(message_view, std::move(platform_handles));
break;
default:
- HandleRemoteError(
- base::StringPrintf("Received message of invalid type %u",
- static_cast<unsigned>(message_view.type()))
- .c_str());
+ HandleRemoteError(StringPrintf("Received message of invalid type %u",
+ static_cast<unsigned>(message_view.type()))
+ .c_str());
break;
}
}
@@ -390,9 +390,10 @@
}
if (!endpoint) {
HandleRemoteError(
- base::StringPrintf(
+ StringPrintf(
"Received a message for nonexistent local destination ID %u",
- static_cast<unsigned>(local_id.value())).c_str());
+ static_cast<unsigned>(local_id.value()))
+ .c_str());
// This is strongly indicative of some problem. However, it's not a fatal
// error, since it may indicate a buggy (or hostile) remote process. Don't
// die even for Debug builds, since handling this properly needs to be
@@ -544,11 +545,12 @@
if (!SendControlMessage(
MessageInTransit::Subtype::CHANNEL_REMOVE_ENDPOINT_ACK, local_id,
remote_id, 0, nullptr)) {
- HandleLocalError(base::StringPrintf(
- "Failed to send message to ack remove remote endpoint "
- "(local ID %u, remote ID %u)",
- static_cast<unsigned>(local_id.value()),
- static_cast<unsigned>(remote_id.value())).c_str());
+ HandleLocalError(
+ StringPrintf("Failed to send message to ack remove remote endpoint "
+ "(local ID %u, remote ID %u)",
+ static_cast<unsigned>(local_id.value()),
+ static_cast<unsigned>(remote_id.value()))
+ .c_str());
}
return true;
@@ -619,11 +621,12 @@
if (!SendControlMessage(
MessageInTransit::Subtype::CHANNEL_ATTACH_AND_RUN_ENDPOINT, local_id,
remote_id, 0, nullptr)) {
- HandleLocalError(base::StringPrintf(
- "Failed to send message to run remote endpoint (local "
- "ID %u, remote ID %u)",
- static_cast<unsigned>(local_id.value()),
- static_cast<unsigned>(remote_id.value())).c_str());
+ HandleLocalError(
+ StringPrintf("Failed to send message to run remote endpoint (local "
+ "ID %u, remote ID %u)",
+ static_cast<unsigned>(local_id.value()),
+ static_cast<unsigned>(remote_id.value()))
+ .c_str());
// TODO(vtl): Should we continue on to |AttachAndRun()|?
}
diff --git a/mojo/edk/system/message_pipe_perftest.cc b/mojo/edk/system/message_pipe_perftest.cc
index 27394c6..e3d77ef 100644
--- a/mojo/edk/system/message_pipe_perftest.cc
+++ b/mojo/edk/system/message_pipe_perftest.cc
@@ -9,7 +9,6 @@
#include "base/bind.h"
#include "base/logging.h"
-#include "base/strings/stringprintf.h"
#include "mojo/edk/platform/scoped_platform_handle.h"
#include "mojo/edk/system/local_message_pipe_endpoint.h"
#include "mojo/edk/system/message_pipe.h"
@@ -19,11 +18,13 @@
#include "mojo/edk/system/test/stopwatch.h"
#include "mojo/edk/test/test_utils.h"
#include "mojo/edk/util/ref_ptr.h"
+#include "mojo/edk/util/string_printf.h"
#include "mojo/public/cpp/system/macros.h"
#include "testing/gtest/include/gtest/gtest.h"
using mojo::platform::ScopedPlatformHandle;
using mojo::util::RefPtr;
+using mojo::util::StringPrintf;
namespace mojo {
namespace system {
@@ -68,9 +69,8 @@
// Have one ping-pong to ensure channel being established.
WriteWaitThenRead(mp);
- std::string test_name =
- base::StringPrintf("IPC_Perf_%dx_%u", message_count_,
- static_cast<unsigned>(message_size_));
+ std::string test_name = StringPrintf("IPC_Perf_%dx_%u", message_count_,
+ static_cast<unsigned>(message_size_));
test::Stopwatch stopwatch;
stopwatch.Start();
diff --git a/mojo/edk/util/BUILD.gn b/mojo/edk/util/BUILD.gn
index 6a08566c..502164b 100644
--- a/mojo/edk/util/BUILD.gn
+++ b/mojo/edk/util/BUILD.gn
@@ -20,6 +20,8 @@
"ref_ptr.h",
"ref_ptr_internal.h",
"scoped_file.h",
+ "string_printf.cc",
+ "string_printf.h",
"thread_annotations.h",
"thread_checker.h",
"waitable_event.cc",
@@ -35,6 +37,7 @@
"cond_var_unittest.cc",
"mutex_unittest.cc",
"ref_counted_unittest.cc",
+ "string_printf_unittest.cc",
"thread_annotations_unittest.cc",
"thread_checker_unittest.cc",
"waitable_event_unittest.cc",
diff --git a/mojo/edk/util/string_printf.cc b/mojo/edk/util/string_printf.cc
new file mode 100644
index 0000000..ba762ee
--- /dev/null
+++ b/mojo/edk/util/string_printf.cc
@@ -0,0 +1,92 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/edk/util/string_printf.h"
+
+#include <assert.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdio.h>
+
+#include <memory>
+
+namespace mojo {
+namespace util {
+namespace {
+
+void StringVAppendfHelper(std::string* dest, const char* format, va_list ap) {
+ // Size of the small stack buffer to use first. This should be kept in sync
+ // with the numbers in StringPrintfTest.StringPrintf_Boundary.
+ constexpr size_t kStackBufferSize = 1024u;
+
+ // First, try with a small buffer on the stack.
+ char stack_buf[kStackBufferSize];
+ // Copy |ap| (which can only be used once), in case we need to retry.
+ va_list ap_copy;
+ va_copy(ap_copy, ap);
+ int result = vsnprintf(stack_buf, kStackBufferSize, format, ap_copy);
+ va_end(ap_copy);
+ if (result < 0) {
+ // As far as I can tell, we'd only get |EOVERFLOW| if the result is so large
+ // that it can't be represented by an |int| (in which case retrying would be
+ // futile), so Chromium's implementation is wrong.
+ return;
+ }
+ // |result| should be the number of characters we need, not including the
+ // terminating null. However, |vsnprintf()| always null-terminates!
+ size_t output_size = static_cast<size_t>(result);
+ // Check if the output fit into our stack buffer. This is "<" not "<=", since
+ // |vsnprintf()| will null-terminate.
+ if (output_size < kStackBufferSize) {
+ // It fit.
+ dest->append(stack_buf, static_cast<size_t>(result));
+ return;
+ }
+
+ // Since we have the required output size, we can just heap allocate that.
+ // (Add 1 because |vsnprintf()| will always null-terminate.)
+ size_t heap_buf_size = output_size + 1u;
+ std::unique_ptr<char[]> heap_buf(new char[heap_buf_size]);
+ result = vsnprintf(heap_buf.get(), heap_buf_size, format, ap);
+ if (result < 0 || static_cast<size_t>(result) > output_size) {
+ assert(false);
+ return;
+ }
+ assert(static_cast<size_t>(result) == output_size);
+ dest->append(heap_buf.get(), static_cast<size_t>(result));
+}
+
+} // namespace
+
+std::string StringPrintf(const char* format, ...) {
+ va_list ap;
+ va_start(ap, format);
+ std::string rv;
+ StringVAppendf(&rv, format, ap);
+ va_end(ap);
+ return rv;
+}
+
+std::string StringVPrintf(const char* format, va_list ap) {
+ std::string rv;
+ StringVAppendf(&rv, format, ap);
+ return rv;
+}
+
+void StringAppendf(std::string* dest, const char* format, ...) {
+ va_list ap;
+ va_start(ap, format);
+ StringVAppendf(dest, format, ap);
+ va_end(ap);
+}
+
+void StringVAppendf(std::string* dest, const char* format, va_list ap) {
+ int old_errno = errno;
+ StringVAppendfHelper(dest, format, ap);
+ errno = old_errno;
+}
+
+} // namespace util
+} // namespace mojo
diff --git a/mojo/edk/util/string_printf.h b/mojo/edk/util/string_printf.h
new file mode 100644
index 0000000..584b5b9
--- /dev/null
+++ b/mojo/edk/util/string_printf.h
@@ -0,0 +1,38 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// |printf()|-like formatting functions that output/append to C++ strings.
+//
+// TODO(vtl): We don't have the |PRINTF_FORMAT()| macros/warnings like
+// Chromium's version -- should we? (I've rarely seen them being useful.)
+
+#ifndef MOJO_EDK_UTIL_STRING_PRINTF_H_
+#define MOJO_EDK_UTIL_STRING_PRINTF_H_
+
+#include <stdarg.h>
+
+#include <string>
+
+#include "mojo/public/c/system/macros.h"
+
+namespace mojo {
+namespace util {
+
+// Formats |printf()|-like input and returns it as an |std::string|.
+std::string StringPrintf(const char* format, ...) MOJO_WARN_UNUSED_RESULT;
+
+// Formats |vprintf()|-like input and returns it as an |std::string|.
+std::string StringVPrintf(const char* format,
+ va_list ap) MOJO_WARN_UNUSED_RESULT;
+
+// Formats |printf()|-like input and appends it to |*dest|.
+void StringAppendf(std::string* dest, const char* format, ...);
+
+// Formats |vprintf()|-like input and appends it to |*dest|.
+void StringVAppendf(std::string* dest, const char* format, va_list ap);
+
+} // namespace util
+} // namespace mojo
+
+#endif // MOJO_EDK_UTIL_STRING_PRINTF_H_
diff --git a/mojo/edk/util/string_printf_unittest.cc b/mojo/edk/util/string_printf_unittest.cc
new file mode 100644
index 0000000..f9faea0
--- /dev/null
+++ b/mojo/edk/util/string_printf_unittest.cc
@@ -0,0 +1,124 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/edk/util/string_printf.h"
+
+#include <errno.h>
+#include <stdarg.h>
+
+#include <string>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mojo {
+namespace util {
+namespace {
+
+// Note: |runnable| can't be a reference since that'd make the behavior of
+// |va_start()| undefined.
+template <typename Runnable>
+std::string VAListHelper(Runnable runnable, ...) {
+ va_list ap;
+ va_start(ap, runnable);
+ std::string rv = runnable(ap);
+ va_end(ap);
+ return rv;
+}
+
+TEST(StringPrintfTest, StringPrintf_Basic) {
+ EXPECT_EQ("", StringPrintf(""));
+ EXPECT_EQ("hello", StringPrintf("hello"));
+ EXPECT_EQ("hello-123", StringPrintf("hello%d", -123));
+ EXPECT_EQ("hello0123FACE", StringPrintf("%s%04d%X", "hello", 123, 0xfaceU));
+}
+
+TEST(StringPrintfTest, StringVPrintf_Basic) {
+ EXPECT_EQ("", VAListHelper([](va_list ap) -> std::string {
+ return StringVPrintf("", ap);
+ }));
+ EXPECT_EQ("hello", VAListHelper([](va_list ap) -> std::string {
+ return StringVPrintf("hello", ap);
+ }));
+ EXPECT_EQ("hello-123", VAListHelper([](va_list ap) -> std::string {
+ return StringVPrintf("hello%d", ap);
+ }, -123));
+ EXPECT_EQ("hello0123FACE", VAListHelper([](va_list ap) -> std::string {
+ return StringVPrintf("%s%04d%X", ap);
+ }, "hello", 123, 0xfaceU));
+}
+
+TEST(StringPrintfTest, StringAppendf_Basic) {
+ {
+ std::string s = "existing";
+ StringAppendf(&s, "");
+ EXPECT_EQ("existing", s);
+ }
+ {
+ std::string s = "existing";
+ StringAppendf(&s, "hello");
+ EXPECT_EQ("existinghello", s);
+ }
+ {
+ std::string s = "existing";
+ StringAppendf(&s, "hello%d", -123);
+ EXPECT_EQ("existinghello-123", s);
+ }
+ {
+ std::string s = "existing";
+ StringAppendf(&s, "%s%04d%X", "hello", 123, 0xfaceU);
+ EXPECT_EQ("existinghello0123FACE", s);
+ }
+}
+
+TEST(StringPrintfTest, StringVAppendf_Basic) {
+ EXPECT_EQ("existing", VAListHelper([](va_list ap) -> std::string {
+ std::string s = "existing";
+ StringVAppendf(&s, "", ap);
+ return s;
+ }));
+ EXPECT_EQ("existinghello", VAListHelper([](va_list ap) -> std::string {
+ std::string s = "existing";
+ StringVAppendf(&s, "hello", ap);
+ return s;
+ }));
+ EXPECT_EQ("existinghello-123", VAListHelper([](va_list ap) -> std::string {
+ std::string s = "existing";
+ StringVAppendf(&s, "hello%d", ap);
+ return s;
+ }, -123));
+ EXPECT_EQ("existinghello0123FACE",
+ VAListHelper([](va_list ap) -> std::string {
+ std::string s = "existing";
+ StringVAppendf(&s, "%s%04d%X", ap);
+ return s;
+ }, "hello", 123, 0xfaceU));
+}
+
+// Generally, we assume that everything forwards to |StringVAppendf()|, so
+// testing |StringPrintf()| more carefully suffices.
+
+TEST(StringPrintfTest, StringPrintf_Boundary) {
+ // Note: The size of strings generated should cover the boundary cases in the
+ // constant |kStackBufferSize| in |StringVAppendf()|.
+ for (size_t i = 800; i < 1200; i++) {
+ std::string stuff(i, 'x');
+ std::string format = stuff + "%d" + "%s" + " world";
+ EXPECT_EQ(stuff + "123" + "hello world",
+ StringPrintf(format.c_str(), 123, "hello"))
+ << i;
+ }
+}
+
+TEST(StringPrintfTest, StringPrintf_VeryBig) {
+ // 4 megabytes of exes (we'll generate 5 times this).
+ std::string stuff(4u << 20u, 'x');
+ std::string format = "%s" + stuff + "%s" + stuff + "%s";
+ EXPECT_EQ(stuff + stuff + stuff + stuff + stuff,
+ StringPrintf(format.c_str(), stuff.c_str(), stuff.c_str(),
+ stuff.c_str()));
+}
+
+} // namespace
+} // namespace util
+} // namespace mojo
