VOM: stats fixes

- double free of m_stat_data_seg
- cleanup typedefs

Change-Id: I6aeb070471b6c5c97ac4379d01bd242136f80091
Signed-off-by: Neale Ranns <nranns@cisco.com>

6 files changed, 88 insertions, 83 deletions

diff --git a/extras/vom/vom/interface.cpp b/extras/vom/vom/interface.cpp
index 504511eabc9..97f7776ebc6 100644
--- a/extras/vom/vom/interface.cpp
+++ b/extras/vom/vom/interface.cpp
@@ -418,7 +418,7 @@ interface::set(const std::string& tag)
 }
 
 void
-interface::set(counter_t count, const std::string& stat_type)
+interface::set(const counter_t& count, const std::string& stat_type)
 {
   if ("rx" == stat_type)
     m_stats.m_rx = count;
diff --git a/extras/vom/vom/interface.hpp b/extras/vom/vom/interface.hpp
index 100c3caf4bc..2f6511eac6d 100644
--- a/extras/vom/vom/interface.hpp
+++ b/extras/vom/vom/interface.hpp
@@ -25,7 +25,6 @@
 #include "vom/route_domain.hpp"
 #include "vom/rpc_cmd.hpp"
 #include "vom/singular_db.hpp"
-#include "vom/stat_client.hpp"
 
 namespace VOM {
 /**
@@ -619,7 +618,7 @@ private:
   /**
    * Set the interface stat
    */
-  void set(counter_t count, const std::string& stat_type);
+  void set(const counter_t& count, const std::string& stat_type);
 
   /**
    * enable the interface stats in the singular instance
diff --git a/extras/vom/vom/stat_client.cpp b/extras/vom/vom/stat_client.cpp
index b348c6c24eb..6406e215193 100644
--- a/extras/vom/vom/stat_client.cpp
+++ b/extras/vom/vom/stat_client.cpp
@@ -17,29 +17,22 @@
 
 namespace VOM {
 
-stat_client::stat_data_t::stat_data_t()
-  : m_name("")
-  , m_type(STAT_DIR_TYPE_ILLEGAL)
-{
-}
-
-stat_client::stat_data_t::stat_data_t(stat_segment_data_t* stat_seg_data)
-  : m_name(stat_seg_data->name)
-  , m_type(stat_seg_data->type)
+stat_client::stat_data_t::stat_data_t(const stat_segment_data_t& stat_seg_data)
+  : m_name(stat_seg_data.name)
+  , m_type(stat_seg_data.type)
 {
   switch (m_type) {
     case STAT_DIR_TYPE_SCALAR_INDEX:
-      m_scalar_value = stat_seg_data->scalar_value;
+      m_scalar_value = stat_seg_data.scalar_value;
       break;
     case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
-      m_simple_counter_vec = stat_seg_data->simple_counter_vec;
+      m_simple_counter_vec = stat_seg_data.simple_counter_vec;
       break;
     case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
-      m_combined_counter_vec =
-        reinterpret_cast<counter_t**>(stat_seg_data->combined_counter_vec);
+      m_combined_counter_vec = stat_seg_data.combined_counter_vec;
       break;
     case STAT_DIR_TYPE_ERROR_INDEX:
-      m_error_Value = stat_seg_data->error_value;
+      m_error_value = stat_seg_data.error_value;
       break;
     case STAT_DIR_TYPE_ILLEGAL:
       break;
@@ -59,22 +52,25 @@ stat_client::stat_data_t::type() const
 }
 
 double
-stat_client::stat_data_t::get_stat_segment_scalar_data()
+stat_client::stat_data_t::get_stat_segment_scalar_data() const
 {
   return m_scalar_value;
 }
+
 uint64_t
-stat_client::stat_data_t::get_stat_segment_error_data()
+stat_client::stat_data_t::get_stat_segment_error_data() const
 {
-  return m_error_Value;
+  return m_error_value;
 }
+
 uint64_t**
-stat_client::stat_data_t::get_stat_segment_simple_counter_data()
+stat_client::stat_data_t::get_stat_segment_simple_counter_data() const
 {
   return m_simple_counter_vec;
 }
-counter_t**
-stat_client::stat_data_t::get_stat_segment_combined_counter_data()
+
+vlib_counter_t**
+stat_client::stat_data_t::get_stat_segment_combined_counter_data() const
 {
   return m_combined_counter_vec;
 }
@@ -83,6 +79,9 @@ stat_client::stat_client(std::string& socket_name)
   : m_socket_name(socket_name)
   , m_patterns()
   , m_stat_connect(0)
+  , m_counter_vec()
+  , m_stat_seg_data(nullptr)
+  , m_stat_data()
 {
   m_patterns.push_back("/if");
 }
@@ -91,6 +90,9 @@ stat_client::stat_client(std::vector<std::string>& pattern)
   : m_socket_name("/run/vpp/stats.sock")
   , m_patterns(pattern)
   , m_stat_connect(0)
+  , m_counter_vec()
+  , m_stat_seg_data(nullptr)
+  , m_stat_data()
 {
 }
 
@@ -99,6 +101,9 @@ stat_client::stat_client(std::string socket_name,
   : m_socket_name(socket_name)
   , m_patterns(patterns)
   , m_stat_connect(0)
+  , m_counter_vec()
+  , m_stat_seg_data(nullptr)
+  , m_stat_data()
 {
 }
 
@@ -106,6 +111,9 @@ stat_client::stat_client()
   : m_socket_name("/run/vpp/stats.sock")
   , m_patterns()
   , m_stat_connect(0)
+  , m_counter_vec()
+  , m_stat_seg_data(nullptr)
+  , m_stat_data()
 {
   m_patterns.push_back("/if");
 }
@@ -127,8 +135,10 @@ stat_client::stat_client(const stat_client& o)
 int
 stat_client::connect()
 {
-  if (stat_segment_connect(m_socket_name.c_str()) == 0)
+  if (stat_segment_connect(m_socket_name.c_str()) == 0) {
     m_stat_connect = 1;
+    ls();
+  }
   return m_stat_connect;
 }
 
@@ -165,19 +175,17 @@ stat_client::ls()
 const stat_client::stat_data_vec_t&
 stat_client::dump()
 {
-  stat_segment_data_t* ssd;
   stat_segment_data_free(m_stat_seg_data);
   if (m_stat_data.size()) {
     m_stat_data.clear();
   }
-  ssd = stat_segment_dump(m_counter_vec);
-  if (!ssd) {
+  m_stat_seg_data = stat_segment_dump(m_counter_vec);
+  if (!m_stat_seg_data) {
     ls();
     return m_stat_data;
   }
-  m_stat_seg_data = ssd;
-  for (int i = 0; i < stat_segment_vec_len(ssd); i++) {
-    stat_data_t sd(&ssd[i]);
+  for (int i = 0; i < stat_segment_vec_len(m_stat_seg_data); i++) {
+    stat_data_t sd(m_stat_seg_data[i]);
     m_stat_data.push_back(sd);
   }
   return m_stat_data;
@@ -186,19 +194,17 @@ stat_client::dump()
 const stat_client::stat_data_vec_t&
 stat_client::dump_entry(uint32_t index)
 {
-  stat_segment_data_t* ssd;
   stat_segment_data_free(m_stat_seg_data);
   if (m_stat_data.size()) {
     m_stat_data.clear();
   }
-  ssd = stat_segment_dump_entry(index);
-  if (!ssd) {
+  m_stat_seg_data = stat_segment_dump_entry(index);
+  if (!m_stat_seg_data) {
     ls();
     return m_stat_data;
   }
-  m_stat_seg_data = ssd;
-  for (int i = 0; i < stat_segment_vec_len(ssd); i++) {
-    stat_data_t sd(&ssd[i]);
+  for (int i = 0; i < stat_segment_vec_len(m_stat_seg_data); i++) {
+    stat_data_t sd(m_stat_seg_data[i]);
     m_stat_data.push_back(sd);
   }
   return m_stat_data;
diff --git a/extras/vom/vom/stat_client.hpp b/extras/vom/vom/stat_client.hpp
index 4da968a8204..40729feef5f 100644
--- a/extras/vom/vom/stat_client.hpp
+++ b/extras/vom/vom/stat_client.hpp
@@ -26,13 +26,6 @@ extern "C" {
 
 namespace VOM {
 
-typedef struct
-{
-  uint64_t packets;
-  uint64_t bytes;
-} counter_t;
-
-typedef uint64_t stat_counter_t;
 /**
  * A representation of a stat client in VPP
  */
@@ -45,14 +38,9 @@ public:
   struct stat_data_t
   {
     /**
-     * stat data constructor
-     */
-    stat_data_t();
-
-    /**
      * stat data custom constructor
      */
-    stat_data_t(stat_segment_data_t* stat_seg_data);
+    stat_data_t(const stat_segment_data_t& stat_seg_data);
 
     /**
      * get name of stat
@@ -67,10 +55,10 @@ public:
     /**
      * Get pointer to actual data
      */
-    double get_stat_segment_scalar_data();
-    uint64_t get_stat_segment_error_data();
-    uint64_t** get_stat_segment_simple_counter_data();
-    counter_t** get_stat_segment_combined_counter_data();
+    double get_stat_segment_scalar_data() const;
+    uint64_t get_stat_segment_error_data() const;
+    uint64_t** get_stat_segment_simple_counter_data() const;
+    vlib_counter_t** get_stat_segment_combined_counter_data() const;
 
   private:
     /**
@@ -89,9 +77,9 @@ public:
     union
     {
       double m_scalar_value;
-      uint64_t m_error_Value;
-      uint64_t** m_simple_counter_vec;
-      counter_t** m_combined_counter_vec;
+      uint64_t m_error_value;
+      counter_t** m_simple_counter_vec;
+      vlib_counter_t** m_combined_counter_vec;
     };
   };
 
@@ -141,43 +129,43 @@ public:
   void disconnect();
 
   /**
-   * Get vector length of VPP style vector
+   * dump all the stats for given pattern
    */
-  int vec_len(void* vec);
+  const stat_data_vec_t& dump();
 
   /**
-   * Free VPP style vector
+   * dump stats for given index in stat directory
    */
-  void vec_free(void* vec);
+  const stat_data_vec_t& dump_entry(uint32_t index);
 
   /**
-   * ls on the stat directory using given pattern
+   * Get vector length of VPP style vector
    */
-  void ls();
+  int vec_len(void* vec);
+
+  double heartbeat();
 
   /**
-   * dump all the stats for given pattern
+   * get index to name of stat
    */
-  const stat_data_vec_t& dump();
+  std::string index_to_name(uint32_t index);
 
+private:
   /**
-   * dump stats for given index in stat directory
+   * Free VPP style vector
    */
-  const stat_data_vec_t& dump_entry(uint32_t index);
+  void vec_free(void* vec);
 
   /**
    * Free stat segment data
    */
   void data_free();
 
-  double heartbeat();
-
   /**
-   * get index to name of stat
+   * ls on the stat directory using given pattern
    */
-  std::string index_to_name(uint32_t index);
+  void ls();
 
-private:
   /**
    * socket name
    */
diff --git a/extras/vom/vom/stat_reader.cpp b/extras/vom/vom/stat_reader.cpp
index 931d0888f71..82e09aedce0 100644
--- a/extras/vom/vom/stat_reader.cpp
+++ b/extras/vom/vom/stat_reader.cpp
@@ -61,7 +61,7 @@ stat_reader::unregisters(const interface& intf)
 void
 stat_reader::read()
 {
-  stat_client::stat_data_vec_t sd = m_client.dump();
+  const stat_client::stat_data_vec_t& sd = m_client.dump();
   for (auto& sde : sd) {
     std::string name;
 
@@ -74,28 +74,29 @@ stat_reader::read()
       case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
       case STAT_DIR_TYPE_ERROR_INDEX:
       case STAT_DIR_TYPE_SCALAR_INDEX:
+      case STAT_DIR_TYPE_ILLEGAL:
         break;
 
-      case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
+      case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED: {
+        vlib_counter_t** data;
+
+        data = sde.get_stat_segment_combined_counter_data();
+
         if (name.find("/if") != std::string::npos)
           name.erase(0, 4);
         for (auto& i : m_stat_itf_indexes) {
-          counter_t count = {.packets = 0, .bytes = 0 };
-          for (int k = 0; k < m_client.vec_len(
-                                sde.get_stat_segment_combined_counter_data());
-               k++) {
-            count.packets +=
-              sde.get_stat_segment_combined_counter_data()[k][i].packets;
-            count.bytes +=
-              sde.get_stat_segment_combined_counter_data()[k][i].bytes;
+          counter_t count;
+
+          for (int k = 0; k < m_client.vec_len(data); k++) {
+            count.packets += data[k][i].packets;
+            count.bytes += data[k][i].bytes;
           }
           std::shared_ptr<interface> itf = interface::find(i);
           if (itf)
             itf->set(count, name);
         }
         break;
-
-      default:;
+      }
     }
   }
 
diff --git a/extras/vom/vom/types.hpp b/extras/vom/vom/types.hpp
index ab214085d16..e3b21add045 100644
--- a/extras/vom/vom/types.hpp
+++ b/extras/vom/vom/types.hpp
@@ -378,6 +378,17 @@ struct l2_address_t
   std::vector<uint8_t> bytes;
 };
 
+struct counter_t
+{
+  counter_t()
+    : packets(0)
+    , bytes(0)
+  {
+  }
+  uint64_t packets;
+  uint64_t bytes;
+};
+
 /**
  * Ostream operator for a MAC address
  */