implement review feedback: avoid copying strings, prefer make_unique, use...

implement review feedback: avoid copying strings, prefer make_unique, use standard naming convention for private members

src/bpftrace.cpp

@@ -355,52 +355,87 @@ std::vector<std::unique_ptr<IPrintable>> BPFtrace::get_arg_values(std::vector<Fi
         switch (arg.type.size)
         {
           case 8:
-            arg_values.emplace_back(new PrintableInt(*(uint64_t*)(arg_data+arg.offset)));
+            arg_values.push_back(
+              std::make_unique<PrintableInt>(
+                *(uint64_t*)(arg_data+arg.offset)));
             break;
           case 4:
-            arg_values.emplace_back(new PrintableInt(*(uint32_t*)(arg_data+arg.offset)));
+            arg_values.push_back(
+              std::make_unique<PrintableInt>(
+                *(uint32_t*)(arg_data+arg.offset)));
             break;
           case 2:
-            arg_values.emplace_back(new PrintableInt(*(uint16_t*)(arg_data+arg.offset)));
+            arg_values.push_back(
+              std::make_unique<PrintableInt>(
+                *(uint16_t*)(arg_data+arg.offset)));
             break;
           case 1:
-            arg_values.emplace_back(new PrintableInt(*(uint8_t*)(arg_data+arg.offset)));
+            arg_values.push_back(
+              std::make_unique<PrintableInt>(
+                *(uint8_t*)(arg_data+arg.offset)));
             break;
           default:
             abort();
         }
         break;
       case Type::string:
-        arg_values.emplace_back(new PrintableString(
-          std::string((char *) arg_data+arg.offset)));
+        arg_values.push_back(
+          std::make_unique<PrintableCString>(
+            (char *) arg_data+arg.offset));
         break;
       case Type::sym:
-        arg_values.emplace_back(new PrintableString(
-          resolve_sym(*(uint64_t*)(arg_data+arg.offset)).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              resolve_sym(*(uint64_t*)(arg_data+arg.offset)))));
         break;
       case Type::usym:
-        arg_values.emplace_back(new PrintableString(
-          resolve_usym(*(uint64_t*)(arg_data+arg.offset), *(uint64_t*)(arg_data+arg.offset + 8)).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              resolve_usym(
+                *(uint64_t*)(arg_data+arg.offset),
+                *(uint64_t*)(arg_data+arg.offset + 8)))));
         break;
       case Type::inet:
-        name = strdup(resolve_inet(*(uint64_t*)(arg_data+arg.offset), *(uint64_t*)(arg_data+arg.offset+8)).c_str());
-        arg_values.push_back((uint64_t)name);
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              resolve_inet(
+                *(uint64_t*)(arg_data+arg.offset),
+                *(uint64_t*)(arg_data+arg.offset+8)))));
         break;
       case Type::username:
-        arg_values.emplace_back(new PrintableString(
-          resolve_uid(*(uint64_t*)(arg_data+arg.offset)).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              resolve_uid(
+                *(uint64_t*)(arg_data+arg.offset)))));
         break;
       case Type::probe:
-        arg_values.emplace_back(new PrintableString(
-          resolve_probe(*(uint64_t*)(arg_data+arg.offset)).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              resolve_probe(
+                *(uint64_t*)(arg_data+arg.offset)))));
         break;
       case Type::stack:
-        arg_values.emplace_back(new PrintableString(
-          get_stack(*(uint64_t*)(arg_data+arg.offset), false, 8).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              get_stack(
+                *(uint64_t*)(arg_data+arg.offset),
+                false,
+                8))));
         break;
       case Type::ustack:
-        arg_values.emplace_back(new PrintableString(
-          get_stack(*(uint64_t*)(arg_data+arg.offset), true, 8).c_str()));
+        arg_values.push_back(
+          std::make_unique<PrintableString>(
+            std::move(
+              get_stack(
+                *(uint64_t*)(arg_data+arg.offset),
+                true,
+                8))));
         break;
       default:
         abort();

src/printf.cpp

@@ -66,12 +66,17 @@ std::string verify_format_string(const std::string &fmt, std::vector<Field> args
 
 uint64_t PrintableString::value()
 {
-  return (uint64_t)_value.c_str();
+  return (uint64_t)value_.c_str();
+}
+
+uint64_t PrintableCString::value()
+{
+  return (uint64_t)value_;
 }
 
 uint64_t PrintableInt::value()
 {
-  return _value;
+  return value_;
 }
 
 } // namespace bpftrace

src/printf.h

@@ -21,19 +21,28 @@ public:
 class PrintableString : public virtual IPrintable
 {
 public:
-  PrintableString(std::string value) : _value(value) { }
+  PrintableString(std::string value) : value_(value) { }
   uint64_t value();
 private:
-  std::string _value;
+  std::string value_;
+};
+
+class PrintableCString : public virtual IPrintable
+{
+public:
+  PrintableCString(char* value) : value_(value) { }
+  uint64_t value();
+private:
+  char* value_;
 };
 
 class PrintableInt : public virtual IPrintable
 {
 public:
-  PrintableInt(uint64_t value) : _value(value) { }
+  PrintableInt(uint64_t value) : value_(value) { }
   uint64_t value();
 private:
-  uint64_t _value;
+  uint64_t value_;
 };