Removed statements with side effects from asserts

In release builds, it is common to define NDEBUG to ignore
asserts. Which means it is a bad idea to call methods with necessary
side-effects inside asserts...

src/bf_lpm_trie/bf_lpm_trie.c

@@ -32,6 +32,8 @@ typedef unsigned char byte_t;
 
 typedef unsigned long value_t;
 
+#define _unused(x) ((void)(x))
+
 typedef struct node_s {
   Pvoid_t PJLarray_branches;
   Pvoid_t PJLarray_prefixes;
@@ -270,14 +272,18 @@ bool bf_lpm_trie_delete(bf_lpm_trie_t *trie, const char *prefix,
   pdata = get_prefix_ptr(current_node, prefix_key);
   if(!pdata) return false;
 
+  int success;
+  _unused(success);
   if(trie->release_memory) {
-    assert(delete_prefix(current_node, prefix_key) == 1);
+    success = delete_prefix(current_node, prefix_key);
+    assert(success == 1);
     current_node->pref_num--;
     while(current_node->pref_num == 0 && current_node->branch_num == 0) {
       node_t *tmp = current_node;
       current_node = current_node->parent;
       if(!current_node) break;
-      assert(delete_branch(current_node, tmp->child_id) == 1);
+      success = delete_branch(current_node, tmp->child_id);
+      assert(success == 1);
       free(tmp);
       current_node->branch_num--;
     }
@@ -286,4 +292,4 @@ bool bf_lpm_trie_delete(bf_lpm_trie_t *trie, const char *prefix,
   return true;
 }
 
-
+#undef _unused

src/bm_sim/action_profile.cpp

@@ -26,6 +26,8 @@
 #include <string>
 #include <vector>
 
+#define _unused(x) ((void)(x))
+
 namespace bm {
 
 void
@@ -243,7 +245,9 @@ ActionProfile::delete_member(mbr_hdl_t mbr) {
     } else if (index_ref_count.get(IndirectIndex::make_mbr_index(mbr)) > 0) {
       rc = MatchErrorCode::MBR_STILL_USED;
     } else {
-      assert(!mbr_handles.release_handle(mbr));
+      int error = mbr_handles.release_handle(mbr);
+      _unused(error);
+      assert(!error);
       num_members--;
     }
   }
@@ -338,7 +342,9 @@ ActionProfile::delete_group(grp_hdl_t grp) {
       for (auto mbr : group_info)
         index_ref_count.decrease(IndirectIndex::make_mbr_index(mbr));
 
-      assert(!grp_handles.release_handle(grp));
+      int error = grp_handles.release_handle(grp);
+      _unused(error);
+      assert(!error);
 
       num_groups--;
     }
@@ -540,14 +546,18 @@ ActionProfile::deserialize(std::istream *in, const P4Objects &objs) {
   (*in) >> num_members;
   for (size_t i = 0; i < num_members; i++) {
     mbr_hdl_t mbr_hdl; (*in) >> mbr_hdl;
-    assert(!mbr_handles.set_handle(mbr_hdl));
+    int error = mbr_handles.set_handle(mbr_hdl);
+    _unused(error);
+    assert(!error);
     action_entries.at(mbr_hdl).deserialize(in, objs);
   }
   index_ref_count.deserialize(in);
   (*in) >> num_groups;
   for (size_t i = 0; i < num_groups; i++) {
     grp_hdl_t grp_hdl; (*in) >> grp_hdl;
-    assert(!grp_handles.set_handle(grp_hdl));
+    int error = grp_handles.set_handle(grp_hdl);
+    _unused(error);
+    assert(!error);
     grp_mgr.insert_group(grp_hdl);
     grp_mgr.at(grp_hdl).deserialize(in);
     for (const auto mbr : grp_mgr.at(grp_hdl))
@@ -584,3 +594,5 @@ ActionProfile::choose_from_group(grp_hdl_t grp, const Packet &pkt) const {
 }
 
 }  // namespace bm
+
+#undef _unused

src/bm_sim/dev_mgr_bmi.cpp

@@ -19,11 +19,13 @@
  */
 
 #include <bm/bm_sim/dev_mgr.h>
+#include <bm/bm_sim/logger.h>
 
-#include <string>
 #include <cassert>
+#include <cstdlib>
 #include <mutex>
 #include <map>
+#include <string>
 
 extern "C" {
 #include "BMI/bmi_port.h"
@@ -43,7 +45,10 @@ class BmiDevMgrImp : public DevMgrIface {
  public:
   BmiDevMgrImp(int device_id,
                std::shared_ptr<TransportIface> notifications_transport) {
-    assert(!bmi_port_create_mgr(&port_mgr));
+    if (bmi_port_create_mgr(&port_mgr)) {
+      Logger::get()->critical("Could not initialize BMI port manager");
+      std::exit(1);
+    }
 
     p_monitor = PortMonitorIface::make_active(device_id,
                                               notifications_transport);
@@ -86,7 +91,8 @@ class BmiDevMgrImp : public DevMgrIface {
 
   void start_() override {
     assert(port_mgr);
-    assert(!bmi_start_mgr(port_mgr));
+    if (bmi_start_mgr(port_mgr))
+      Logger::get()->critical("Could not start BMI port manager");
   }
 
   ReturnCode set_packet_handler_(const PacketHandler &handler, void *cookie)
@@ -95,7 +101,10 @@ class BmiDevMgrImp : public DevMgrIface {
     function_t * const*ptr_fun = handler.target<function_t *>();
     assert(ptr_fun);
     assert(*ptr_fun);
-    assert(!bmi_set_packet_handler(port_mgr, *ptr_fun, cookie));
+    if (bmi_set_packet_handler(port_mgr, *ptr_fun, cookie)) {
+      Logger::get()->critical("Could not set BMI packet handler");
+      return ReturnCode::ERROR;
+    }
     return ReturnCode::SUCCESS;
   }
 

src/bm_sim/lookup_structures.cpp

@@ -32,6 +32,8 @@
 
 #include "lpm_trie.h"
 
+#define _unused(x) ((void)(x))
+
 namespace bm {
 
 namespace {  // anonymous
@@ -172,7 +174,9 @@ class TernaryCache {
     if (it != cache.end()) return false;
     if (cache.size() == S) {  // cache is full
       auto evict = entries.back();
-      assert(cache.erase(evict.key) == 1);
+      int success = cache.erase(evict.key);
+      _unused(success);
+      assert(success == 1);
       entries.pop_back();
     }
     entries.emplace_front(handle, key_data);
@@ -542,3 +546,5 @@ LookupStructureFactory::create_for_range(size_t size, size_t nbytes_key) {
 }
 
 }  // namespace bm
+
+#undef _unused

src/bm_sim/match_units.cpp

@@ -35,6 +35,8 @@
 
 #include "utils.h"
 
+#define _unused(x) ((void)(x))
+
 namespace bm {
 
 #define HANDLE_VERSION(h) (h >> 24)
@@ -1143,7 +1145,9 @@ MatchUnitGeneric<K, V>::deserialize_(std::istream *in, const P4Objects &objs) {
   for (size_t i = 0; i < this->num_entries; i++) {
     Entry entry;
     internal_handle_t handle_; (*in) >> handle_;
-    assert(!this->handles.set_handle(handle_));
+    int error = this->handles.set_handle(handle_);
+    _unused(error);
+    assert(!error);
     uint32_t version; (*in) >> version;
     std::string key_hex; (*in) >> key_hex;
     entry.key.data = ByteContainer(key_hex);
@@ -1193,3 +1197,5 @@ template class
 MatchUnitGeneric<RangeMatchKey, ActionProfile::IndirectIndex>;
 
 }  // namespace bm
+
+#undef _unused

src/bm_sim/parser_error.cpp

@@ -60,7 +60,7 @@ ErrorCodeMap::add_core() {
                           Core::StackOutOfBounds, Core::HeaderTooShort,
                           Core::ParserTimeout}) {
     auto name = core_to_name(core);
-    if (!exists(name)) assert(add(name, max_v++));
+    if (!exists(name)) add(name, max_v++);
   }
 }
 

src/bm_sim/simple_pre.cpp

@@ -28,6 +28,8 @@
 
 #include "jsoncpp/json.h"
 
+#define _unused(x) ((void)(x))
+
 namespace bm {
 
 McSimplePre::McReturnCode
@@ -153,9 +155,13 @@ McSimplePre::mc_node_destroy(const l1_hdl_t l1_hdl) {
   }
   rid = l1_entry.rid;
   l2_entries.erase(l1_entry.l2_hdl);
-  assert(!l2_handles.release_handle(l1_entry.l2_hdl));
+  int error;
+  _unused(error);
+  error = l2_handles.release_handle(l1_entry.l2_hdl);
+  assert(!error);
   l1_entries.erase(l1_hdl);
-  assert(!l1_handles.release_handle(l1_hdl));
+  error = l1_handles.release_handle(l1_hdl);
+  assert(!error);
   Logger::get()->debug("node destroyed for rid {}", rid);
   return SUCCESS;
 }
@@ -297,3 +303,5 @@ McSimplePre::node_dissociate(MgidEntry *mgid_entry, l1_hdl_t l1_hdl) {
 }
 
 }  // namespace bm
+
+#undef _unused

src/bm_sim/switch.cpp

@@ -35,6 +35,8 @@
 
 #include "md5.h"
 
+#define _unused(x) ((void)(x))
+
 namespace bm {
 
 static void
@@ -317,7 +319,9 @@ SwitchWContexts::swap_configs() {
   {
     std::unique_lock<std::mutex> config_lock(config_mutex);
     if (!config_loaded) config_loaded = true;
-    assert(do_swap() == 0);
+    int error = do_swap();
+    _unused(error);
+    assert(!error);
   }
   config_loaded_cv.notify_one();
   return ErrorCode::SUCCESS;
@@ -522,3 +526,5 @@ Switch::new_packet(int ingress_port, packet_id_t id, int ingress_length,
 }
 
 }  // namespace bm
+
+#undef _unused