common/BackTrace: refactor into Clib and Py implementations

Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/src/common/BackTrace.cc b/src/common/BackTrace.cc
index 42a8da2aebd1eb2e2a7cd43ce5f830e1e9730b4d..03105d64ad9c1beb5f96baf981682566ec1b30cd 100644 (file)
--- a/src/common/BackTrace.cc
+++ b/src/common/BackTrace.cc
@@ -11,7 +11,7 @@
 
 namespace ceph {
 
-void BackTrace::print(std::ostream& out) const
+void ClibBackTrace::print(std::ostream& out) const
 {
   out << " " << pretty_version_to_str() << std::endl;
   for (size_t i = skip; i < size; i++) {
@@ -19,7 +19,7 @@ void BackTrace::print(std::ostream& out) const
   }
 }
 
-void BackTrace::dump(Formatter *f) const
+void ClibBackTrace::dump(Formatter *f) const
 {
   f->open_array_section("backtrace");
   for (size_t i = skip; i < size; i++) {
@@ -29,7 +29,7 @@ void BackTrace::dump(Formatter *f) const
   f->close_section();
 }
 
-std::string BackTrace::demangle(const char* name)
+std::string ClibBackTrace::demangle(const char* name)
 {
   // find the parentheses and address offset surrounding the mangled name
 #ifdef __FreeBSD__
@@ -71,4 +71,20 @@ std::string BackTrace::demangle(const char* name)
   }
 }
 
+void PyBackTrace::dump(Formatter *f) const
+{
+  f->open_array_section("backtrace");
+  for (auto& i : strings) {
+    f->dump_string("frame", i);
+  }
+  f->close_section();
+}
+
+void PyBackTrace::print(std::ostream& out) const
+{
+  for (auto& i : strings) {
+    out << i << std::endl;
+  }
+}
+
 }

diff --git a/src/common/BackTrace.h b/src/common/BackTrace.h
index ce3912f52cd00efb0e3b960856bfbd64cc09a281..d89b34d3e8e49c700618d55c502c7e66ccf7dee1 100644 (file)
--- a/src/common/BackTrace.h
+++ b/src/common/BackTrace.h
@@ -19,6 +19,18 @@ namespace ceph {
 class Formatter;
 
 struct BackTrace {
+  virtual ~BackTrace() {}
+  virtual void print(std::ostream& out) const = 0;
+  virtual void dump(Formatter *f) const = 0;
+};
+
+inline std::ostream& operator<<(std::ostream& out, const BackTrace& bt) {
+  bt.print(out);
+  return out;
+}
+
+
+struct ClibBackTrace : public BackTrace {
   const static int max = 32;
 
   int skip;
@@ -26,20 +38,9 @@ struct BackTrace {
   size_t size;
   char **strings;
 
-  std::list<std::string> src_strings;
-
-  explicit BackTrace(std::list<std::string>& s)
-    : skip(0),
-      size(s.size()) {
-    src_strings = s;
-    strings = (char **)malloc(sizeof(*strings) * src_strings.size());
-    unsigned i = 0;
-    for (auto& s : src_strings) {
-      strings[i++] = (char *)s.c_str();
-    }
-  }
-  explicit BackTrace(int s) : skip(s) {
+  explicit ClibBackTrace(int s) {
 #ifdef HAVE_EXECINFO_H
+    skip = s;
     size = backtrace(array, max);
     strings = backtrace_symbols(array, size);
 #else
@@ -48,22 +49,29 @@ struct BackTrace {
     strings = nullptr;
 #endif
   }
-  ~BackTrace() {
+  ~ClibBackTrace() {
     free(strings);
   }
 
-  BackTrace(const BackTrace& other);
-  const BackTrace& operator=(const BackTrace& other);
+  ClibBackTrace(const ClibBackTrace& other);
+  const ClibBackTrace& operator=(const ClibBackTrace& other);
+
+  void print(std::ostream& out) const override;
+  void dump(Formatter *f) const override;
 
-  void print(std::ostream& out) const;
-  void dump(Formatter *f) const;
   static std::string demangle(const char* name);
 };
 
-inline std::ostream& operator<<(std::ostream& out, const BackTrace& bt) {
-  bt.print(out);
-  return out;
-}
+
+struct PyBackTrace : public BackTrace {
+  std::list<std::string> strings;
+
+  explicit PyBackTrace(std::list<std::string>& s) : strings(s) {}
+
+  void dump(Formatter *f) const override;
+  void print(std::ostream& out) const override;
+};
+
 
 }
 

diff --git a/src/common/assert.cc b/src/common/assert.cc
index a663c16e907d1395f4c3908dda9e88dcf282537b..7fb4c2d726b074609a10336541be3ab6fff6c96a 100644 (file)
--- a/src/common/assert.cc
+++ b/src/common/assert.cc
@@ -59,7 +59,7 @@ namespace ceph {
 
     // TODO: get rid of this memory allocation.
     ostringstream oss;
-    oss << BackTrace(1);
+    oss << ClibBackTrace(1);
     dout_emergency(oss.str());
 
     if (g_assert_context) {
@@ -126,7 +126,7 @@ namespace ceph {
                       sizeof(g_assert_thread_name));
 
     BufAppender ba(g_assert_msg, sizeof(g_assert_msg));
-    BackTrace *bt = new BackTrace(1);
+    BackTrace *bt = new ClibBackTrace(1);
     ba.printf("%s: In function '%s' thread %llx time %s\n"
             "%s: %d: FAILED ceph_assert(%s)\n",
             file, func, (unsigned long long)pthread_self(), tss.str().c_str(),
@@ -171,7 +171,7 @@ namespace ceph {
     ceph_pthread_getname(pthread_self(), g_assert_thread_name,
                       sizeof(g_assert_thread_name));
 
-    BackTrace *bt = new BackTrace(1);
+    BackTrace *bt = new ClibBackTrace(1);
     snprintf(g_assert_msg, sizeof(g_assert_msg),
              "%s: In function '%s' thread %llx time %s\n"
             "%s: %d: ceph_abort_msg(\"%s\")\n", file, func,
@@ -214,7 +214,7 @@ namespace ceph {
                       sizeof(g_assert_thread_name));
 
     BufAppender ba(g_assert_msg, sizeof(g_assert_msg));
-    BackTrace *bt = new BackTrace(1);
+    BackTrace *bt = new ClibBackTrace(1);
     ba.printf("%s: In function '%s' thread %llx time %s\n"
              "%s: %d: abort()\n",
              file, func, (unsigned long long)pthread_self(), tss.str().c_str(),

diff --git a/src/common/cmdparse.cc b/src/common/cmdparse.cc
index 1d72320ef51b7859ac6da71ae35a5e0e8d787967..7e66869097d81dd0b58bc65ff9a6c222a12697d1 100644 (file)
--- a/src/common/cmdparse.cc
+++ b/src/common/cmdparse.cc
@@ -438,7 +438,7 @@ handle_bad_get(CephContext *cct, const string& k, const char *tname)
   lderr(cct) << errstr.str() << dendl;
 
   ostringstream oss;
-  oss << BackTrace(1);
+  oss << ClibBackTrace(1);
   lderr(cct) << oss.str() << dendl;
 
   if (status == 0)

diff --git a/src/common/lockdep.cc b/src/common/lockdep.cc
index 18d0cde09c8894738dea7f29b6268088e8bf49d7..aa7d9e0f0e42e2f5964839d107896d746b456f12 100644 (file)
--- a/src/common/lockdep.cc
+++ b/src/common/lockdep.cc
@@ -297,7 +297,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace,
       if (!recursive) {
        lockdep_dout(0) << "\n";
        *_dout << "recursive lock of " << name << " (" << id << ")\n";
-       auto bt = new ceph::BackTrace(BACKTRACE_SKIP);
+       auto bt = new ceph::ClibBackTrace(BACKTRACE_SKIP);
        bt->print(*_dout);
        if (p->second) {
          *_dout << "\npreviously locked at\n";
@@ -312,7 +312,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace,
 
       // did we just create a cycle?
       if (does_follow(id, p->first)) {
-        auto bt = new ceph::BackTrace(BACKTRACE_SKIP);
+        auto bt = new ceph::ClibBackTrace(BACKTRACE_SKIP);
        lockdep_dout(0) << "new dependency " << lock_names[p->first]
                << " (" << p->first << ") -> " << name << " (" << id << ")"
                << " creates a cycle at\n";
@@ -338,7 +338,7 @@ int lockdep_will_lock(const char *name, int id, bool force_backtrace,
       } else {
        ceph::BackTrace* bt = NULL;
         if (force_backtrace || lockdep_force_backtrace()) {
-          bt = new ceph::BackTrace(BACKTRACE_SKIP);
+          bt = new ceph::ClibBackTrace(BACKTRACE_SKIP);
         }
         follows[p->first].set(id);
         follows_bt[p->first][id] = bt;
@@ -363,7 +363,7 @@ int lockdep_locked(const char *name, int id, bool force_backtrace)
 
   lockdep_dout(20) << "_locked " << name << dendl;
   if (force_backtrace || lockdep_force_backtrace())
-    held[p][id] = new ceph::BackTrace(BACKTRACE_SKIP);
+    held[p][id] = new ceph::ClibBackTrace(BACKTRACE_SKIP);
   else
     held[p][id] = 0;
 out:

diff --git a/src/global/signal_handler.cc b/src/global/signal_handler.cc
index f4e41a5cf970237fb27d4b8d4a113802a6d8bda3..055763eee4691eccfaa05559f63c4b1a75eb874b 100644 (file)
--- a/src/global/signal_handler.cc
+++ b/src/global/signal_handler.cc
@@ -322,7 +322,7 @@ static void handle_oneshot_fatal_signal(int signum)
   // TODO: don't use an ostringstream here. It could call malloc(), which we
   // don't want inside a signal handler.
   // Also fix the backtrace code not to allocate memory.
-  BackTrace bt(1);
+  ClibBackTrace bt(1);
   ostringstream oss;
   bt.print(oss);
   dout_emergency(oss.str());

diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc
index 399fa47e62c76f2c4622744f2f8cc7e07a955664..0c143d5721309cc4d854633577bc2cb6b0019ed7 100644 (file)
--- a/src/os/filestore/FileStore.cc
+++ b/src/os/filestore/FileStore.cc
@@ -4134,7 +4134,7 @@ public:
   }
 
   void finish(int r) override {
-    BackTrace *bt = new BackTrace(1);
+    BackTrace *bt = new ClibBackTrace(1);
     generic_dout(-1) << "FileStore: sync_entry timed out after "
           << m_commit_timeo << " seconds.\n";
     bt->print(*_dout);

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 4fa4e634d23d2eb2ac60f37acc83e7bbfce241c4..aaa7c9324eaf76811f23fc7510bcd92d63b8f64a 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -136,7 +136,7 @@ uint64_t PG::get_with_id()
   ref++;
   std::lock_guard l(_ref_id_lock);
   uint64_t id = ++_ref_id;
-  BackTrace bt(0);
+  ClibBackTrace bt(0);
   stringstream ss;
   bt.print(ss);
   lgeneric_subdout(cct, refs, 5) << "PG::get " << this << " " << info.pgid

diff --git a/src/test/common/test_back_trace.cc b/src/test/common/test_back_trace.cc
index 43f9bc559aad41dd2155a71e65a553aadef5a558..97db3268671c1aea974fd6f163fddfdf9335a9c9 100644 (file)
--- a/src/test/common/test_back_trace.cc
+++ b/src/test/common/test_back_trace.cc
@@ -16,7 +16,7 @@
 std::string foo()
 {
   std::ostringstream oss;
-  oss << ceph::BackTrace(1);
+  oss << ceph::ClibBackTrace(1);
   return oss.str();
 }