src: add C++-style sprintf utility

Add an utility that handles C++-style strings and objects well.
nodejs · addaleax · Jan 21, 2020 · Jan 21, 2020 · Jan 21, 2020 · Jan 21, 2020
commit a35bf6bd24939e0ba3f4d7b2d3bafbdf3dcdf2df
diff --git a/node.gyp b/node.gyp
@@ -608,6 +608,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,86 @@
+#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) {
+  return format;
+}
+
+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, '%');
+  if (p == nullptr) return format;
+  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.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_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_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"

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"
@@ -1063,7 +1063,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/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/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,43 @@ 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(), 8u);
+  EXPECT_GE(SPrintF("%p", p).size(), 8u);
+
+  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");
+
+  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);
+}