src: add C++-style sprintf utility

Add an utility that handles C++-style strings and objects well. PR-URL: #31446 Fixes: #28761 Fixes: #31218 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · Mar 17, 2020 · 7bdd29f · 7bdd29f
1 parent 393b48e
commit 7bdd29f
Show file tree

Hide file tree

Showing 18 changed files with 165 additions and 18 deletions.
diff --git a/node.gyp b/node.gyp
@@ -609,6 +609,7 @@
         'src/connect_wrap.h',
         'src/connection_wrap.h',
         'src/debug_utils.h',
+        'src/debug_utils-inl.h',
         'src/env.h',
         'src/env-inl.h',
         'src/handle_wrap.h',

diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h
@@ -0,0 +1,90 @@
+#ifndef SRC_DEBUG_UTILS_INL_H_
+#define SRC_DEBUG_UTILS_INL_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "debug_utils.h"
+
+#include <type_traits>
+
+namespace node {
+
+struct ToStringHelper {
+  template <typename T>
+  static std::string Convert(
+      const T& value,
+      std::string(T::* to_string)() const = &T::ToString) {
+    return (value.*to_string)();
+  }
+  template <typename T,
+            typename test_for_number = typename std::
+                enable_if<std::is_arithmetic<T>::value, bool>::type,
+            typename dummy = bool>
+  static std::string Convert(const T& value) { return std::to_string(value); }
+  static std::string Convert(const char* value) { return value; }
+  static std::string Convert(const std::string& value) { return value; }
+  static std::string Convert(bool value) { return value ? "true" : "false"; }
+};
+
+template <typename T>
+std::string ToString(const T& value) {
+  return ToStringHelper::Convert(value);
+}
+
+inline std::string SPrintFImpl(const char* format) {
+  const char* p = strchr(format, '%');
+  if (LIKELY(p == nullptr)) return format;
+  CHECK_EQ(p[1], '%');  // Only '%%' allowed when there are no arguments.
+
+  return std::string(format, p + 1) + SPrintFImpl(p + 2);
+}
+
+template <typename Arg, typename... Args>
+std::string COLD_NOINLINE SPrintFImpl(  // NOLINT(runtime/string)
+    const char* format, Arg&& arg, Args&&... args) {
+  const char* p = strchr(format, '%');
+  CHECK_NOT_NULL(p);  // If you hit this, you passed in too many arguments.
+  std::string ret(format, p);
+  // Ignore long / size_t modifiers
+  while (strchr("lz", *++p) != nullptr) {}
+  switch (*p) {
+    case '%': {
+      return ret + '%' + SPrintFImpl(p + 1,
+                                     std::forward<Arg>(arg),
+                                     std::forward<Args>(args)...);
+    }
+    default: {
+      return ret + '%' + SPrintFImpl(p,
+                                     std::forward<Arg>(arg),
+                                     std::forward<Args>(args)...);
+    }
+    case 'd':
+    case 'i':
+    case 'u':
+    case 's': ret += ToString(arg); break;
+    case 'p': {
+      CHECK(std::is_pointer<typename std::remove_reference<Arg>::type>::value);
+      char out[20];
+      int n = snprintf(out,
+                       sizeof(out),
+                       "%p",
+                       *reinterpret_cast<const void* const*>(&arg));
+      CHECK_GE(n, 0);
+      ret += out;
+      break;
+    }
+  }
+  return ret + SPrintFImpl(p + 1, std::forward<Args>(args)...);
+}
+
+template <typename... Args>
+std::string COLD_NOINLINE SPrintF(  // NOLINT(runtime/string)
+    const char* format, Args&&... args) {
+  return SPrintFImpl(format, std::forward<Args>(args)...);
+}
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_DEBUG_UTILS_INL_H_
diff --git a/src/debug_utils.cc b/src/debug_utils.cc
@@ -1,4 +1,4 @@
-#include "debug_utils.h"
+#include "debug_utils-inl.h"  // NOLINT(build/include)
 #include "env-inl.h"
 
 #ifdef __POSIX__

diff --git a/src/debug_utils.h b/src/debug_utils.h
@@ -22,14 +22,26 @@
 
 namespace node {
 
+template <typename T>
+inline std::string ToString(const T& value);
+
+// C++-style variant of sprintf() that:
+// - Returns an std::string
+// - Handles \0 bytes correctly
+// - Supports %p and %s. %d, %i and %u are aliases for %s.
+// - Accepts any class that has a ToString() method for stringification.
+template <typename... Args>
+inline std::string SPrintF(const char* format, Args&&... args);
+
 template <typename... Args>
 inline void FORCE_INLINE Debug(Environment* env,
                                DebugCategory cat,
                                const char* format,
                                Args&&... args) {
   if (!UNLIKELY(env->debug_enabled(cat)))
     return;
-  fprintf(stderr, format, std::forward<Args>(args)...);
+  std::string out = SPrintF(format, std::forward<Args>(args)...);
+  fwrite(out.data(), out.size(), 1, stderr);
 }
 
 inline void FORCE_INLINE Debug(Environment* env,

diff --git a/src/inspector_io.cc b/src/inspector_io.cc
@@ -4,7 +4,7 @@
 #include "inspector/main_thread_interface.h"
 #include "inspector/node_string.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "node.h"
 #include "node_crypto.h"
 #include "node_internals.h"

diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc
@@ -1,6 +1,6 @@
 #include "inspector_profiler.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "diagnosticfilename-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_file.h"

diff --git a/src/node.cc b/src/node.cc
@@ -23,7 +23,7 @@
 
 // ========== local headers ==========
 
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_binding.h"

diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -1,5 +1,5 @@
 #include "aliased_buffer.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node.h"
 #include "node_buffer.h"
@@ -1967,7 +1967,7 @@ std::string Http2Stream::diagnostic_name() const {
 
 // Notify the Http2Stream that a new block of HEADERS is being processed.
 void Http2Stream::StartHeaders(nghttp2_headers_category category) {
-  Debug(this, "starting headers, category: %d", id_, category);
+  Debug(this, "starting headers, category: %d", category);
   CHECK(!this->IsDestroyed());
   session_->DecrementCurrentSessionMemory(current_headers_length_);
   current_headers_length_ = 0;
@@ -2220,7 +2220,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
     req_wrap->Done(UV_EOF);
     return 0;
   }
-  Debug(this, "queuing %d buffers to send", id_, nbufs);
+  Debug(this, "queuing %d buffers to send", nbufs);
   for (size_t i = 0; i < nbufs; ++i) {
     // Store the req_wrap on the last write info in the queue, so that it is
     // only marked as finished once all buffers associated with it are finished.

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -1,7 +1,7 @@
 #include "node_messaging.h"
 
 #include "async_wrap-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_contextify.h"
 #include "node_buffer.h"

diff --git a/src/node_platform.cc b/src/node_platform.cc
@@ -2,7 +2,7 @@
 #include "node_internals.h"
 
 #include "env-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include <algorithm>  // find_if(), find(), move()
 #include <cmath>  // llround()
 #include <memory>  // unique_ptr(), shared_ptr(), make_shared()

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -1,6 +1,6 @@
 #include "env-inl.h"
 #include "node_report.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "diagnosticfilename-inl.h"
 #include "node_internals.h"
 #include "node_metadata.h"

diff --git a/src/node_wasi.cc b/src/node_wasi.cc
@@ -1,6 +1,6 @@
 #include "env-inl.h"
 #include "base_object-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_mem-inl.h"
 #include "util-inl.h"
@@ -1062,7 +1062,7 @@ void WASI::PathFilestatGet(const FunctionCallbackInfo<Value>& args) {
   CHECK_TO_TYPE_OR_RETURN(args, args[4], Uint32, buf_ptr);
   ASSIGN_OR_RETURN_UNWRAP(&wasi, args.This());
   Debug(wasi,
-        "path_filestat_get(%d, %d, %d, %d, %d)\n",
+        "path_filestat_get(%d, %d, %d)\n",
         fd,
         path_ptr,
         path_len);

diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc
@@ -21,7 +21,7 @@
 
 #include <algorithm>
 
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "node_errors.h"
 #include "node_internals.h"

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -1,5 +1,5 @@
 #include "node_worker.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_errors.h"
 #include "node_buffer.h"

diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc
@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "spawn_sync.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 #include "node_internals.h"
 #include "string_bytes.h"

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -21,7 +21,7 @@
 
 #include "tls_wrap.h"
 #include "async_wrap-inl.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_buffer.h"  // Buffer
 #include "node_crypto.h"  // SecureContext

diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc
@@ -3,7 +3,7 @@
 #include <string>
 #include "trace_event.h"
 #include "tracing/node_trace_buffer.h"
-#include "debug_utils.h"
+#include "debug_utils-inl.h"
 #include "env-inl.h"
 
 namespace node {

diff --git a/test/cctest/test_util.cc b/test/cctest/test_util.cc
@@ -1,4 +1,6 @@
 #include "util-inl.h"
+#include "debug_utils-inl.h"
+#include "env-inl.h"
 #include "gtest/gtest.h"
 
 TEST(UtilTest, ListHead) {
@@ -250,3 +252,45 @@ TEST(UtilTest, MaybeStackBuffer) {
     EXPECT_TRUE(buf.IsInvalidated());
   }
 }
+
+TEST(UtilTest, SPrintF) {
+  using node::SPrintF;
+
+  // %d, %u and %s all do the same thing. The actual C++ type is used to infer
+  // the right representation.
+  EXPECT_EQ(SPrintF("%s", false), "false");
+  EXPECT_EQ(SPrintF("%s", true), "true");
+  EXPECT_EQ(SPrintF("%d", true), "true");
+  EXPECT_EQ(SPrintF("%u", true), "true");
+  EXPECT_EQ(SPrintF("%d", 10000000000LL), "10000000000");
+  EXPECT_EQ(SPrintF("%d", -10000000000LL), "-10000000000");
+  EXPECT_EQ(SPrintF("%u", 10000000000LL), "10000000000");
+  EXPECT_EQ(SPrintF("%u", -10000000000LL), "-10000000000");
+  EXPECT_EQ(SPrintF("%i", 10), "10");
+  EXPECT_EQ(SPrintF("%d", 10), "10");
+
+  EXPECT_EQ(atof(SPrintF("%s", 0.5).c_str()), 0.5);
+  EXPECT_EQ(atof(SPrintF("%s", -0.5).c_str()), -0.5);
+
+  void (*fn)() = []() {};
+  void* p = reinterpret_cast<void*>(&fn);
+  EXPECT_GE(SPrintF("%p", fn).size(), 4u);
+  EXPECT_GE(SPrintF("%p", p).size(), 4u);
+
+  const std::string foo = "foo";
+  const char* bar = "bar";
+  EXPECT_EQ(SPrintF("%s %s", foo, "bar"), "foo bar");
+  EXPECT_EQ(SPrintF("%s %s", foo, bar), "foo bar");
+
+  EXPECT_EQ(SPrintF("[%% %s %%]", foo), "[% foo %]");
+
+  struct HasToString {
+    std::string ToString() const {
+      return "meow";
+    }
+  };
+  EXPECT_EQ(SPrintF("%s", HasToString{}), "meow");
+
+  const std::string with_zero = std::string("a") + '\0' + 'b';
+  EXPECT_EQ(SPrintF("%s", with_zero), with_zero);
+}