Move implementation of USDT::Argument into cc file

To avoid exposing the implementation of new/delete of the Argument
class, move it out of the header file. This also requires making private
the std::string members, so that they cannot be assigned to directly.
Let the ArgumentParser assign to them as a friend, and expose const
functions to access them as const only from the client.

Also, convert non-const reference usages to pass-by-pointer.

diff --git a/src/cc/usdt.h b/src/cc/usdt.h
index 8d0e606..852f303 100644 (file)
--- a/src/cc/usdt.h
+++ b/src/cc/usdt.h
@@ -15,19 +15,31 @@
  */
 #pragma once
 
-#include <unordered_map>
 #include <string>
+#include <unordered_map>
 
 namespace USDT {
 
-struct Argument {
-  int arg_size;
-  int constant;
-  int deref_offset;
-  std::string deref_ident;
-  std::string register_name;
+class ArgumentParser;
+
+class Argument {
+ private:
+  int arg_size_;
+  int constant_;
+  int deref_offset_;
+  std::string deref_ident_;
+  std::string register_name_;
+
+ public:
+  Argument();
+  ~Argument();
 
-  Argument() : arg_size(0), constant(0), deref_offset(0) {}
+  const std::string &deref_ident() const { return deref_ident_; }
+  const std::string &register_name() const { return register_name_; }
+  int arg_size() const { return arg_size_; }
+  int constant() const { return constant_; }
+  int deref_offset() const { return deref_offset_; }
+  friend class ArgumentParser;
 };
 
 class ArgumentParser {
@@ -35,18 +47,18 @@ class ArgumentParser {
   ssize_t cur_pos_;
 
  protected:
-  virtual bool validate_register(const std::string &reg, int &reg_size) = 0;
+  virtual bool validate_register(const std::string &reg, int *reg_size) = 0;
 
-  ssize_t parse_number(ssize_t pos, int &number);
-  ssize_t parse_identifier(ssize_t pos, std::string &ident);
-  ssize_t parse_register(ssize_t pos, Argument &dest);
-  ssize_t parse_expr(ssize_t pos, Argument &dest);
-  ssize_t parse_1(ssize_t pos, Argument &dest);
+  ssize_t parse_number(ssize_t pos, int *number);
+  ssize_t parse_identifier(ssize_t pos, std::string *ident);
+  ssize_t parse_register(ssize_t pos, Argument *dest);
+  ssize_t parse_expr(ssize_t pos, Argument *dest);
+  ssize_t parse_1(ssize_t pos, Argument *dest);
 
   void print_error(ssize_t pos);
 
  public:
-  bool parse(Argument &dest);
+  bool parse(Argument *dest);
   bool done() { return arg_[cur_pos_] == '\0'; }
 
   ArgumentParser(const char *arg) : arg_(arg), cur_pos_(0) {}
@@ -54,7 +66,7 @@ class ArgumentParser {
 
 class ArgumentParser_x64 : public ArgumentParser {
   static const std::unordered_map<std::string, int> registers_;
-  bool validate_register(const std::string &reg, int &reg_size);
+  bool validate_register(const std::string &reg, int *reg_size);
 
  public:
   ArgumentParser_x64(const char *arg) : ArgumentParser(arg) {}

diff --git a/src/cc/usdt_args.cc b/src/cc/usdt_args.cc
index 4807859..a38a2d2 100644 (file)
--- a/src/cc/usdt_args.cc
+++ b/src/cc/usdt_args.cc
@@ -14,54 +14,56 @@
  * limitations under the License.
  */
 #include <unordered_map>
-#include <stdio.h>
-
 #include "usdt.h"
 
 namespace USDT {
 
-ssize_t ArgumentParser::parse_number(ssize_t pos, int &number) {
+Argument::Argument() : arg_size_(0), constant_(0), deref_offset_(0) {}
+
+Argument::~Argument() {}
+
+ssize_t ArgumentParser::parse_number(ssize_t pos, int *number) {
   char *endp;
-  number = strtol(arg_ + pos, &endp, 0);
+  *number = strtol(arg_ + pos, &endp, 0);
   return endp - arg_;
 }
 
-ssize_t ArgumentParser::parse_identifier(ssize_t pos, std::string &ident) {
+ssize_t ArgumentParser::parse_identifier(ssize_t pos, std::string *ident) {
   if (isalpha(arg_[pos]) || arg_[pos] == '_') {
     ssize_t start = pos++;
     while (isalnum(arg_[pos]) || arg_[pos] == '_') pos++;
-    ident.assign(arg_ + start, pos - start);
+    ident->assign(arg_ + start, pos - start);
   }
   return pos;
 }
 
-ssize_t ArgumentParser::parse_register(ssize_t pos, Argument &dest) {
+ssize_t ArgumentParser::parse_register(ssize_t pos, Argument *dest) {
   ssize_t start = pos++;
   if (arg_[start] != '%')
     return -start;
   while (isalnum(arg_[pos])) pos++;
-  dest.register_name.assign(arg_ + start, pos - start);
-  if (!validate_register(dest.register_name, dest.arg_size))
+  dest->register_name_.assign(arg_ + start, pos - start);
+  if (!validate_register(dest->register_name(), &dest->arg_size_))
     return -start;
   return pos;
 }
 
-ssize_t ArgumentParser::parse_expr(ssize_t pos, Argument &dest) {
+ssize_t ArgumentParser::parse_expr(ssize_t pos, Argument *dest) {
   if (arg_[pos] == '$')
-    return parse_number(pos + 1, dest.constant);
+    return parse_number(pos + 1, &dest->constant_);
 
   if (arg_[pos] == '%')
     return parse_register(pos, dest);
 
   if (isdigit(arg_[pos]) || arg_[pos] == '-') {
-    pos = parse_number(pos, dest.deref_offset);
+    pos = parse_number(pos, &dest->deref_offset_);
     if (arg_[pos] == '+') {
-      pos = parse_identifier(pos + 1, dest.deref_ident);
-      if (dest.deref_ident.empty())
+      pos = parse_identifier(pos + 1, &dest->deref_ident_);
+      if (dest->deref_ident().empty())
         return -pos;
     }
   } else {
-    pos = parse_identifier(pos, dest.deref_ident);
+    pos = parse_identifier(pos, &dest->deref_ident_);
   }
 
   if (arg_[pos] != '(')
@@ -74,12 +76,12 @@ ssize_t ArgumentParser::parse_expr(ssize_t pos, Argument &dest) {
   return (arg_[pos] == ')') ? pos + 1 : -pos;
 }
 
-ssize_t ArgumentParser::parse_1(ssize_t pos, Argument &dest) {
+ssize_t ArgumentParser::parse_1(ssize_t pos, Argument *dest) {
   if (isdigit(arg_[pos]) || arg_[pos] == '-') {
     int asize;
-    ssize_t m = parse_number(pos, asize);
+    ssize_t m = parse_number(pos, &asize);
     if (arg_[m] == '@') {
-      dest.arg_size = asize;
+      dest->arg_size_ = asize;
       return parse_expr(m + 1, dest);
     }
   }
@@ -93,7 +95,7 @@ void ArgumentParser::print_error(ssize_t pos) {
   fputc('\n', stderr);
 }
 
-bool ArgumentParser::parse(Argument &dest) {
+bool ArgumentParser::parse(Argument *dest) {
   if (done())
     return false;
 
@@ -126,12 +128,12 @@ const std::unordered_map<std::string, int> ArgumentParser_x64::registers_ = {
     {"%al", 1},  {"%bl", 1},  {"%cl", 1},  {"%dl", 1}};
 
 bool ArgumentParser_x64::validate_register(const std::string &reg,
-                                           int &reg_size) {
+                                           int *reg_size) {
   auto it = registers_.find(reg);
   if (it == registers_.end())
     return false;
-  if (reg_size == 0)
-    reg_size = it->second;
+  if (*reg_size == 0)
+    *reg_size = it->second;
   return true;
 }
 }

diff --git a/tests/cc/test_c_api.cc b/tests/cc/test_c_api.cc
index 5592b84..46a6cd4 100644 (file)
--- a/tests/cc/test_c_api.cc
+++ b/tests/cc/test_c_api.cc
@@ -1,7 +1,7 @@
-#include <stdint.h>
-#include <unistd.h>
 #include <dlfcn.h>
+#include <stdint.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "bcc_elf.h"
 #include "bcc_proc.h"

diff --git a/tests/cc/test_usdt_args.cc b/tests/cc/test_usdt_args.cc
index 8b4fa63..d150ef2 100644 (file)
--- a/tests/cc/test_usdt_args.cc
+++ b/tests/cc/test_usdt_args.cc
@@ -6,12 +6,12 @@ static void verify_register(USDT::ArgumentParser_x64 &parser, int arg_size,
                             const std::string &register_name, int constant,
                             int deref_offset, const std::string &deref_ident) {
   USDT::Argument arg;
-  REQUIRE(parser.parse(arg));
-  REQUIRE(arg.arg_size == arg_size);
-  REQUIRE(arg.register_name == register_name);
-  REQUIRE(arg.constant == constant);
-  REQUIRE(arg.deref_offset == deref_offset);
-  REQUIRE(arg.deref_ident == deref_ident);
+  REQUIRE(parser.parse(&arg));
+  REQUIRE(arg.arg_size() == arg_size);
+  REQUIRE(arg.register_name() == register_name);
+  REQUIRE(arg.constant() == constant);
+  REQUIRE(arg.deref_offset() == deref_offset);
+  REQUIRE(arg.deref_ident() == deref_ident);
 }
 
 TEST_CASE("test usdt argument parsing", "[usdt]") {