From fe344b45bb2d4ea79c19989af5f006a8671fa175 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Mon, 3 Jul 2017 10:55:51 -0700 Subject: [PATCH] Removed statements with side effects from asserts (#402) 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... --- include/Makefile.am | 1 + include/bm/bm_sim/_assert.h | 39 +++++++++++++++++++++ include/bm/bm_sim/actions.h | 7 ++-- include/bm/bm_sim/nn.h | 1 + src/bf_lpm_trie/bf_lpm_trie.c | 12 +++++-- src/bm_apps/nn.h | 1 + src/bm_runtime/Standard_server.cpp | 5 +-- src/bm_sim/Makefile.am | 1 + src/bm_sim/_assert.cpp | 34 ++++++++++++++++++ src/bm_sim/action_profile.cpp | 19 +++++++--- src/bm_sim/debugger.cpp | 6 ++-- src/bm_sim/dev_mgr_bmi.cpp | 17 ++++++--- src/bm_sim/lookup_structures.cpp | 5 ++- src/bm_sim/match_tables.cpp | 5 +++ src/bm_sim/match_units.cpp | 5 ++- src/bm_sim/parser_error.cpp | 2 +- src/bm_sim/simple_pre.cpp | 9 +++-- src/bm_sim/switch.cpp | 5 ++- tests/stress_tests/stress_utils.h | 1 + tests/stress_tests/test_LPM_match_1.cpp | 13 +++---- tests/stress_tests/test_exact_match_1.cpp | 13 +++---- tests/stress_tests/test_ternary_match_1.cpp | 13 +++---- tests/test_p4objects.cpp | 3 ++ tests/test_runtime_iface.cpp | 3 +- tests/test_switch.cpp | 2 +- tests/utils.cpp.in | 4 ++- 26 files changed, 179 insertions(+), 47 deletions(-) create mode 100644 include/bm/bm_sim/_assert.h create mode 100644 src/bm_sim/_assert.cpp diff --git a/include/Makefile.am b/include/Makefile.am index 2455f6dc7..e80be1e72 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -18,6 +18,7 @@ bm/bm_runtime/bm_runtime.h endif nobase_include_HEADERS += \ +bm/bm_sim/_assert.h \ bm/bm_sim/action_entry.h \ bm/bm_sim/action_profile.h \ bm/bm_sim/actions.h \ diff --git a/include/bm/bm_sim/_assert.h b/include/bm/bm_sim/_assert.h new file mode 100644 index 000000000..969b2c0a9 --- /dev/null +++ b/include/bm/bm_sim/_assert.h @@ -0,0 +1,39 @@ +/* Copyright 2013-present Barefoot Networks, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Antonin Bas (antonin@barefootnetworks.com) + * + */ + +#ifndef BM_BM_SIM__ASSERT_H_ +#define BM_BM_SIM__ASSERT_H_ + +// An assert that cannot be removed with NDEBUG + +namespace bm { + +[[ noreturn ]] void _assert(const char* expr, const char* file, int line); + +} // namespace bm + +#define _BM_ASSERT(expr) \ + ((expr) ? (void)0 : bm::_assert(#expr, __FILE__, __LINE__)) + +#define _BM_UNREACHABLE(msg) bm::_assert(msg, __FILE__, __LINE__) + +#define _BM_UNUSED(x) ((void)x) + +#endif // BM_BM_SIM__ASSERT_H_ diff --git a/include/bm/bm_sim/actions.h b/include/bm/bm_sim/actions.h index 102dd1339..1977aa016 100644 --- a/include/bm/bm_sim/actions.h +++ b/include/bm/bm_sim/actions.h @@ -84,6 +84,7 @@ #include "named_p4object.h" #include "expressions.h" #include "stateful.h" +#include "_assert.h" namespace bm { @@ -278,7 +279,7 @@ Data &ActionParam::to(ActionEngineState *state) const { return state->phv.get_header_stack(stack_field.header_stack).get_last() .get_field(stack_field.field_offset); default: - assert(0); + _BM_UNREACHABLE("Default switch case should not be reachable"); } } @@ -314,7 +315,7 @@ const Data &ActionParam::to(ActionEngineState *state) const { return state->phv.get_header_stack(stack_field.header_stack).get_last() .get_field(stack_field.field_offset); default: - assert(0); + _BM_UNREACHABLE("Default switch case should not be reachable"); } } @@ -363,7 +364,7 @@ StackIface &ActionParam::to(ActionEngineState *state) const { case HEADER_UNION_STACK: return state->phv.get_header_union_stack(header_union_stack); default: - assert(0); + _BM_UNREACHABLE("Default switch case should not be reachable"); } } diff --git a/include/bm/bm_sim/nn.h b/include/bm/bm_sim/nn.h index 4a2c28a65..c44f0b56d 100644 --- a/include/bm/bm_sim/nn.h +++ b/include/bm/bm_sim/nn.h @@ -95,6 +95,7 @@ namespace nn inline ~socket () { int rc = nn_close (s); + (void) rc; assert (rc == 0); } diff --git a/src/bf_lpm_trie/bf_lpm_trie.c b/src/bf_lpm_trie/bf_lpm_trie.c index c94c33054..c51741b44 100644 --- a/src/bf_lpm_trie/bf_lpm_trie.c +++ b/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 diff --git a/src/bm_apps/nn.h b/src/bm_apps/nn.h index 4a2c28a65..c44f0b56d 100644 --- a/src/bm_apps/nn.h +++ b/src/bm_apps/nn.h @@ -95,6 +95,7 @@ namespace nn inline ~socket () { int rc = nn_close (s); + (void) rc; assert (rc == 0); } diff --git a/src/bm_runtime/Standard_server.cpp b/src/bm_runtime/Standard_server.cpp index fb9750325..6baeb8554 100644 --- a/src/bm_runtime/Standard_server.cpp +++ b/src/bm_runtime/Standard_server.cpp @@ -20,8 +20,9 @@ #include -#include +#include #include +#include #include @@ -94,7 +95,7 @@ class StandardHandler : virtual public StandardIf { case MatchErrorCode::ERROR: return TableOperationErrorCode::ERROR; default: - assert(0 && "invalid error code"); + _BM_UNREACHABLE("Default switch case should not be reachable"); } } diff --git a/src/bm_sim/Makefile.am b/src/bm_sim/Makefile.am index 9874d6189..e8e75f877 100644 --- a/src/bm_sim/Makefile.am +++ b/src/bm_sim/Makefile.am @@ -14,6 +14,7 @@ AM_CFLAGS += $(COVERAGE_FLAGS) noinst_LTLIBRARIES = libbmsim.la libbmsim_la_SOURCES = \ +_assert.cpp \ action_profile.cpp \ actions.cpp \ ageing.cpp \ diff --git a/src/bm_sim/_assert.cpp b/src/bm_sim/_assert.cpp new file mode 100644 index 000000000..6d9f1fa02 --- /dev/null +++ b/src/bm_sim/_assert.cpp @@ -0,0 +1,34 @@ +/* Copyright 2013-present Barefoot Networks, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Antonin Bas (antonin@barefootnetworks.com) + * + */ + +#include + +#include +#include + +namespace bm { + +void _assert(const char* expr, const char* file, int line) { + std::cerr << "Assertion '" << expr << "' failed, file '" << file + << "' line '" << line << "'.\n"; + std::abort(); +} + +} // namespace bm diff --git a/src/bm_sim/action_profile.cpp b/src/bm_sim/action_profile.cpp index dc14b3c8d..9c0a18135 100644 --- a/src/bm_sim/action_profile.cpp +++ b/src/bm_sim/action_profile.cpp @@ -18,6 +18,7 @@ * */ +#include #include #include #include @@ -243,7 +244,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); + _BM_UNUSED(error); + assert(!error); num_members--; } } @@ -338,7 +341,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); + _BM_UNUSED(error); + assert(!error); num_groups--; } @@ -448,6 +453,7 @@ ActionProfile::get_members() const { size_t idx = 0; for (const auto h : mbr_handles) { MatchErrorCode rc = get_member_(h, &members[idx++]); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); } return members; @@ -478,6 +484,7 @@ ActionProfile::get_groups() const { size_t idx = 0; for (const auto h : grp_handles) { MatchErrorCode rc = get_group_(h, &groups[idx++]); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); } return groups; @@ -540,14 +547,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); + _BM_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); + _BM_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)) diff --git a/src/bm_sim/debugger.cpp b/src/bm_sim/debugger.cpp index cf970263b..d798da779 100644 --- a/src/bm_sim/debugger.cpp +++ b/src/bm_sim/debugger.cpp @@ -375,13 +375,11 @@ DebuggerNN::packet_in_(const PacketId &packet_id, int port) { wait_for_resume_packet_in(lock); } - auto it = packet_registers_map.find(packet_id); - assert(it == packet_registers_map.end()); + assert(packet_registers_map.find(packet_id) == packet_registers_map.end()); packet_registers_map.emplace_hint(packet_registers_map.end(), packet_id, PacketRegisters()); - auto it2 = packet_ctr_stacks.find(packet_id); - assert(it2 == packet_ctr_stacks.end()); + assert(packet_ctr_stacks.find(packet_id) == packet_ctr_stacks.end()); packet_ctr_stacks.emplace_hint(packet_ctr_stacks.end(), packet_id, std::vector()); diff --git a/src/bm_sim/dev_mgr_bmi.cpp b/src/bm_sim/dev_mgr_bmi.cpp index a94bc561f..6663d47b8 100644 --- a/src/bm_sim/dev_mgr_bmi.cpp +++ b/src/bm_sim/dev_mgr_bmi.cpp @@ -19,11 +19,13 @@ */ #include +#include -#include #include +#include #include #include +#include extern "C" { #include "BMI/bmi_port.h" @@ -43,7 +45,10 @@ class BmiDevMgrImp : public DevMgrIface { public: BmiDevMgrImp(int device_id, std::shared_ptr 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(); 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; } diff --git a/src/bm_sim/lookup_structures.cpp b/src/bm_sim/lookup_structures.cpp index 985bc099b..195f6c0d6 100644 --- a/src/bm_sim/lookup_structures.cpp +++ b/src/bm_sim/lookup_structures.cpp @@ -19,6 +19,7 @@ * */ +#include #include #include @@ -172,7 +173,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); + _BM_UNUSED(success); + assert(success == 1); entries.pop_back(); } entries.emplace_front(handle, key_data); diff --git a/src/bm_sim/match_tables.cpp b/src/bm_sim/match_tables.cpp index 54cc4d66b..ee1c698d8 100644 --- a/src/bm_sim/match_tables.cpp +++ b/src/bm_sim/match_tables.cpp @@ -18,6 +18,7 @@ * */ +#include #include #include #include @@ -529,6 +530,7 @@ MatchTable::get_entries() const { for (auto it = match_unit->handles_begin(); it != match_unit->handles_end(); it++) { MatchErrorCode rc = get_entry_(*it, &entries[idx++]); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); } @@ -574,6 +576,7 @@ MatchTable::set_default_entry(const ActionFn *action_fn, ActionData action_data, bool is_const) { assert(!const_default_entry); auto rc = set_default_action(action_fn, std::move(action_data)); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); const_default_entry = is_const; } @@ -824,6 +827,7 @@ MatchTableIndirect::get_entries() const { for (auto it = match_unit->handles_begin(); it != match_unit->handles_end(); it++) { MatchErrorCode rc = get_entry_(*it, &entries[idx++]); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); } @@ -1044,6 +1048,7 @@ MatchTableIndirectWS::get_entries() const { for (auto it = match_unit->handles_begin(); it != match_unit->handles_end(); it++) { MatchErrorCode rc = get_entry_(*it, &entries[idx++]); + _BM_UNUSED(rc); assert(rc == MatchErrorCode::SUCCESS); } diff --git a/src/bm_sim/match_units.cpp b/src/bm_sim/match_units.cpp index 7caf48849..ab43c944b 100644 --- a/src/bm_sim/match_units.cpp +++ b/src/bm_sim/match_units.cpp @@ -18,6 +18,7 @@ * */ +#include #include #include #include @@ -1143,7 +1144,9 @@ MatchUnitGeneric::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_); + _BM_UNUSED(error); + assert(!error); uint32_t version; (*in) >> version; std::string key_hex; (*in) >> key_hex; entry.key.data = ByteContainer(key_hex); diff --git a/src/bm_sim/parser_error.cpp b/src/bm_sim/parser_error.cpp index 35652d988..0fa04b3a2 100644 --- a/src/bm_sim/parser_error.cpp +++ b/src/bm_sim/parser_error.cpp @@ -60,7 +60,7 @@ ErrorCodeMap::add_core() { Core::StackOutOfBounds, Core::OverwritingHeader, 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++); } } diff --git a/src/bm_sim/simple_pre.cpp b/src/bm_sim/simple_pre.cpp index 1f5055ed7..a5a52e7c2 100644 --- a/src/bm_sim/simple_pre.cpp +++ b/src/bm_sim/simple_pre.cpp @@ -19,6 +19,7 @@ * */ +#include #include #include @@ -153,9 +154,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; + _BM_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; } diff --git a/src/bm_sim/switch.cpp b/src/bm_sim/switch.cpp index aa807dac6..420b3414c 100644 --- a/src/bm_sim/switch.cpp +++ b/src/bm_sim/switch.cpp @@ -18,6 +18,7 @@ * */ +#include #include #include #include @@ -317,7 +318,9 @@ SwitchWContexts::swap_configs() { { std::unique_lock config_lock(config_mutex); if (!config_loaded) config_loaded = true; - assert(do_swap() == 0); + int error = do_swap(); + _BM_UNUSED(error); + assert(!error); } config_loaded_cv.notify_one(); return ErrorCode::SUCCESS; diff --git a/tests/stress_tests/stress_utils.h b/tests/stress_tests/stress_utils.h index b92bc1753..589f44cc0 100644 --- a/tests/stress_tests/stress_utils.h +++ b/tests/stress_tests/stress_utils.h @@ -21,6 +21,7 @@ #ifndef TESTS_STRESS_TESTS_STRESS_UTILS_H_ #define TESTS_STRESS_TESTS_STRESS_UTILS_H_ +#include #include #include diff --git a/tests/stress_tests/test_LPM_match_1.cpp b/tests/stress_tests/test_LPM_match_1.cpp index a103f0438..4b7bbbabf 100644 --- a/tests/stress_tests/test_LPM_match_1.cpp +++ b/tests/stress_tests/test_LPM_match_1.cpp @@ -95,9 +95,10 @@ bm::MatchErrorCode add_entry_3(SwitchTest *sw, const ethernet_t &hdr) { &handle); } -bool check_rc(bm::MatchErrorCode rc) { - return (rc == bm::MatchErrorCode::SUCCESS || - rc == bm::MatchErrorCode::DUPLICATE_ENTRY); +void check_rc(bm::MatchErrorCode rc) { + _BM_UNUSED(rc); + assert(rc == bm::MatchErrorCode::SUCCESS || + rc == bm::MatchErrorCode::DUPLICATE_ENTRY); } } // namespace @@ -121,9 +122,9 @@ int main(int argc, char* argv[]) { for (const auto &pkt : packets) { ethernet_t *hdr = reinterpret_cast(pkt->data()); assert(ntohs(hdr->etherType) == 0x0800); // check for IPv4 ethertype - assert(check_rc(add_entry_1(&sw, *hdr))); - assert(check_rc(add_entry_2(&sw, *hdr))); - assert(check_rc(add_entry_3(&sw, *hdr))); + check_rc(add_entry_1(&sw, *hdr)); + check_rc(add_entry_2(&sw, *hdr)); + check_rc(add_entry_3(&sw, *hdr)); } auto parser = sw.get_parser("parser"); diff --git a/tests/stress_tests/test_exact_match_1.cpp b/tests/stress_tests/test_exact_match_1.cpp index a34d211f4..5eb91a2dd 100644 --- a/tests/stress_tests/test_exact_match_1.cpp +++ b/tests/stress_tests/test_exact_match_1.cpp @@ -76,9 +76,10 @@ bm::MatchErrorCode add_entry_3(SwitchTest *sw, const ethernet_t &hdr) { &handle); } -bool check_rc(bm::MatchErrorCode rc) { - return (rc == bm::MatchErrorCode::SUCCESS || - rc == bm::MatchErrorCode::DUPLICATE_ENTRY); +void check_rc(bm::MatchErrorCode rc) { + _BM_UNUSED(rc); + assert(rc == bm::MatchErrorCode::SUCCESS || + rc == bm::MatchErrorCode::DUPLICATE_ENTRY); } } // namespace @@ -101,9 +102,9 @@ int main(int argc, char* argv[]) { for (const auto &pkt : packets) { ethernet_t *hdr = reinterpret_cast(pkt->data()); assert(ntohs(hdr->etherType) == 0x0800); // check for IPv4 ethertype - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_1(&sw, *hdr))); - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_2(&sw, *hdr))); - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_3(&sw, *hdr))); + if (rgen.get_bool(p_add)) check_rc(add_entry_1(&sw, *hdr)); + if (rgen.get_bool(p_add)) check_rc(add_entry_2(&sw, *hdr)); + if (rgen.get_bool(p_add)) check_rc(add_entry_3(&sw, *hdr)); } auto parser = sw.get_parser("parser"); diff --git a/tests/stress_tests/test_ternary_match_1.cpp b/tests/stress_tests/test_ternary_match_1.cpp index a1cac3aaf..223cfe309 100644 --- a/tests/stress_tests/test_ternary_match_1.cpp +++ b/tests/stress_tests/test_ternary_match_1.cpp @@ -90,9 +90,10 @@ bm::MatchErrorCode add_entry_3(SwitchTest *sw, const ethernet_t &hdr) { &handle); } -bool check_rc(bm::MatchErrorCode rc) { - return (rc == bm::MatchErrorCode::SUCCESS || - rc == bm::MatchErrorCode::DUPLICATE_ENTRY); +void check_rc(bm::MatchErrorCode rc) { + _BM_UNUSED(rc); + assert(rc == bm::MatchErrorCode::SUCCESS || + rc == bm::MatchErrorCode::DUPLICATE_ENTRY); } } // namespace @@ -116,9 +117,9 @@ int main(int argc, char* argv[]) { for (const auto &pkt : packets) { ethernet_t *hdr = reinterpret_cast(pkt->data()); assert(ntohs(hdr->etherType) == 0x0800); // check for IPv4 ethertype - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_1(&sw, *hdr))); - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_2(&sw, *hdr))); - if (rgen.get_bool(p_add)) assert(check_rc(add_entry_3(&sw, *hdr))); + if (rgen.get_bool(p_add)) check_rc(add_entry_1(&sw, *hdr)); + if (rgen.get_bool(p_add)) check_rc(add_entry_2(&sw, *hdr)); + if (rgen.get_bool(p_add)) check_rc(add_entry_3(&sw, *hdr)); } auto parser = sw.get_parser("parser"); diff --git a/tests/test_p4objects.cpp b/tests/test_p4objects.cpp index 716fdd4ae..798a9cc91 100644 --- a/tests/test_p4objects.cpp +++ b/tests/test_p4objects.cpp @@ -344,10 +344,13 @@ TEST(P4Objects, HeaderStackArith) { ASSERT_NO_THROW(h1_f1.get_int()); ASSERT_NO_THROW(h0_f1.get_int()); + (void) h0_f2; (void) h1_f2; +#ifndef NDEBUG if (!WITH_VALGRIND) { ASSERT_DEATH(h0_f2.get_int(), "Assertion .*failed"); ASSERT_DEATH(h1_f2.get_int(), "Assertion .*failed"); } +#endif } TEST(P4Objects, Errors) { diff --git a/tests/test_runtime_iface.cpp b/tests/test_runtime_iface.cpp index 10c77457f..563badd75 100644 --- a/tests/test_runtime_iface.cpp +++ b/tests/test_runtime_iface.cpp @@ -22,6 +22,7 @@ #include +#include #include #include @@ -50,7 +51,7 @@ class RuntimeIfaceTest : public ::testing::Test { virtual void SetUp() { // load JSON fs::path json_path = fs::path(testdata_dir) / fs::path(test_json); - assert(sw.init_objects(json_path.string()) == 0); + _BM_ASSERT(sw.init_objects(json_path.string()) == 0); } // virtual void TearDown() { } diff --git a/tests/test_switch.cpp b/tests/test_switch.cpp index ecc1e6774..f81253188 100644 --- a/tests/test_switch.cpp +++ b/tests/test_switch.cpp @@ -83,7 +83,7 @@ TEST(Switch, GetConfig) { std::ifstream fs(md5_path.string()); for (size_t i = 0; i < md5.size(); i++) { char c1, c2; - assert(fs.get(c1)); assert(fs.get(c2)); + ASSERT_TRUE(fs.get(c1)); ASSERT_TRUE(fs.get(c2)); md5[i] = (char2digit(c1) << 4) | char2digit(c2); } } diff --git a/tests/utils.cpp.in b/tests/utils.cpp.in index bade1426e..d4404f012 100644 --- a/tests/utils.cpp.in +++ b/tests/utils.cpp.in @@ -23,6 +23,8 @@ #include +#include + #include #include @@ -52,7 +54,7 @@ class CLIWrapperImp { } ~CLIWrapperImp() { - assert(pclose(CLI_f) == 0); + _BM_ASSERT(pclose(CLI_f) == 0); } void send_cmd(const std::string &cmd) {