From fa63f8c376408e31c3c6cf4d35af9e078f366e7f Mon Sep 17 00:00:00 2001
From: thorstenhater <24411438+thorstenhater@users.noreply.github.com>
Date: Mon, 2 Aug 2021 15:59:05 +0200
Subject: [PATCH] Bug/assorted static analysis (#1615)
* Fix failure to return value in `py_mech_cat_value_iterator `.
* String butchered by formatting.
* Join unnecessary separation of string literals in `arborio/cabelio.cpp`
* Add missing throw in s_expr code.
* Return value, not reference, in `merge_iterator::operator++(int)`.
* Check for self-assignment in `mechanism_catalogue`.
* Initialize all members of `mechanism`.
* Fix index check order in `multi_event_stream` debug output.
* Use coordinator_ from base class in `memory::array`.
---
.../backends/multicore/multi_event_stream.hpp | 4 +--
arbor/include/arbor/mechanism.hpp | 4 +--
arbor/mechcat.cpp | 4 ++-
arbor/memory/array.hpp | 28 ++++++++-----------
arbor/s_expr.cpp | 2 +-
arbor/util/mergeview.hpp | 2 +-
arborio/cableio.cpp | 8 +++---
python/mechanism.cpp | 2 +-
8 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/arbor/backends/multicore/multi_event_stream.hpp b/arbor/backends/multicore/multi_event_stream.hpp
index efc4d482..6fe2a61a 100644
--- a/arbor/backends/multicore/multi_event_stream.hpp
+++ b/arbor/backends/multicore/multi_event_stream.hpp
@@ -172,14 +172,14 @@ public:
out << "\n[";
unsigned i = 0;
for (unsigned ev_i = 0; ev_i<n_ev; ++ev_i) {
- while (m.span_end_[i]<=ev_i && i<n) ++i;
+ while (i<n && m.span_end_[i]<=ev_i) ++i;
out << (i<n? util::strprintf(" % 7d ", i): " ?");
}
out << "]\n[";
i = 0;
for (unsigned ev_i = 0; ev_i<n_ev; ++ev_i) {
- while (m.span_end_[i]<=ev_i && i<n) ++i;
+ while (i<n && m.span_end_[i]<=ev_i) ++i;
bool discarded = i<n && m.span_begin_[i]>ev_i;
bool marked = i<n && m.mark_[i]>ev_i;
diff --git a/arbor/include/arbor/mechanism.hpp b/arbor/include/arbor/mechanism.hpp
index 40fab8aa..aececa1a 100644
--- a/arbor/include/arbor/mechanism.hpp
+++ b/arbor/include/arbor/mechanism.hpp
@@ -30,7 +30,7 @@ public:
using size_type = fvm_size_type;
mechanism(const arb_mechanism_type m,
- const arb_mechanism_interface& i): mech_{m}, iface_{i} {
+ const arb_mechanism_interface& i): mech_{m}, iface_{i}, ppack_{} {
if (mech_.abi_version != ARB_MECH_ABI_VERSION) throw unsupported_abi_error{mech_.abi_version};
}
mechanism() = default;
@@ -70,7 +70,7 @@ public:
arb_mechanism_type mech_;
arb_mechanism_interface iface_;
arb_mechanism_ppack ppack_;
- arb_value_type** time_ptr_ptr;
+ arb_value_type** time_ptr_ptr = nullptr;
};
struct mechanism_layout {
diff --git a/arbor/mechcat.cpp b/arbor/mechcat.cpp
index 0b39ffd2..3109f17e 100644
--- a/arbor/mechcat.cpp
+++ b/arbor/mechcat.cpp
@@ -523,7 +523,9 @@ mechanism_catalogue::mechanism_catalogue(const mechanism_catalogue& other):
{}
mechanism_catalogue& mechanism_catalogue::operator=(const mechanism_catalogue& other) {
- state_.reset(new catalogue_state(*other.state_));
+ if (this != &other) {
+ state_.reset(new catalogue_state(*other.state_));
+ }
return *this;
}
diff --git a/arbor/memory/array.hpp b/arbor/memory/array.hpp
index 59fd4c39..1bbdbc36 100644
--- a/arbor/memory/array.hpp
+++ b/arbor/memory/array.hpp
@@ -87,12 +87,12 @@ template <typename T, typename Coord>
class array :
public array_view<T, Coord> {
public:
- using value_type = T;
- using base = array_view<value_type, Coord>;
- using view_type = array_view<value_type, Coord>;
- using const_view_type = const_array_view<value_type, Coord>;
+ using base = array_view<T, Coord>;
+ using view_type = base;
+ using const_view_type = const_array_view<T, Coord>;
- using coordinator_type = typename Coord::template rebind<value_type>;
+ using typename base::value_type;
+ using typename base::coordinator_type;
using typename base::size_type;
using typename base::difference_type;
@@ -152,7 +152,7 @@ public:
<< "\n this " << util::pretty_printer<array>::print(*this)
<< "\n other " << util::pretty_printer<array>::print(other) << "\n";
#endif
- coordinator_.copy(const_view_type(other), view_type(*this));
+ base::coordinator_.copy(const_view_type(other), view_type(*this));
}
// move constructor
@@ -179,7 +179,7 @@ public:
<< "\n this " << util::pretty_printer<array>::print(*this)
<< "\n other " << util::pretty_printer<Other>::print(other) << std::endl;
#endif
- coordinator_.copy(typename Other::const_view_type(other), view_type(*this));
+ base::coordinator_.copy(typename Other::const_view_type(other), view_type(*this));
}
array& operator=(const array& other) {
@@ -188,10 +188,10 @@ public:
<< "\n this " << util::pretty_printer<array>::print(*this)
<< "\n other " << util::pretty_printer<array>::print(other) << "\n";
#endif
- coordinator_.free(*this);
+ base::coordinator_.free(*this);
auto ptr = coordinator_type().allocate(other.size());
base::reset(ptr.data(), other.size());
- coordinator_.copy(const_view_type(other), view_type(*this));
+ base::coordinator_.copy(const_view_type(other), view_type(*this));
return *this;
}
@@ -211,7 +211,7 @@ public:
std::cerr << util::red("~") + util::green("array()")
<< "\n this " << util::pretty_printer<array>::print(*this) << "\n";
#endif
- coordinator_.free(*this);
+ base::coordinator_.free(*this);
}
template <
@@ -232,7 +232,7 @@ public:
arb_assert(&*b+(e-b)==&*e);
using V = typename std::iterator_traits<iterator>::value_type;
- coordinator_.copy(const_array_view<V, host_coordinator<V, aligned_allocator<V>>>(&*b, e-b), view_type(*this));
+ base::coordinator_.copy(const_array_view<V, host_coordinator<V, aligned_allocator<V>>>(&*b, e-b), view_type(*this));
}
// use the accessors provided by array_view
@@ -241,16 +241,12 @@ public:
using base::operator();
const coordinator_type& coordinator() const {
- return coordinator_;
+ return base::coordinator_;
}
using base::size;
using base::alignment;
-
-private:
-
- coordinator_type coordinator_;
};
} // namespace memory
diff --git a/arbor/s_expr.cpp b/arbor/s_expr.cpp
index d6e0ef58..3de94915 100644
--- a/arbor/s_expr.cpp
+++ b/arbor/s_expr.cpp
@@ -280,7 +280,7 @@ private:
token string() {
using namespace std::string_literals;
if (*stream_ != '"') {
- s_expr_lexer_error(
+ throw s_expr_lexer_error(
"Lexer attempting to read string without opening \"", loc());
}
diff --git a/arbor/util/mergeview.hpp b/arbor/util/mergeview.hpp
index 4af3ab21..8145b648 100644
--- a/arbor/util/mergeview.hpp
+++ b/arbor/util/mergeview.hpp
@@ -135,7 +135,7 @@ public:
return *this;
}
- merge_iterator& operator++(int) {
+ merge_iterator operator++(int) {
auto me = *this;
++*this;
return me;
diff --git a/arborio/cableio.cpp b/arborio/cableio.cpp
index 4440cfa7..5fde6ff6 100644
--- a/arborio/cableio.cpp
+++ b/arborio/cableio.cpp
@@ -583,11 +583,11 @@ eval_map named_evals{
"'threshold-detector' with 1 argument (threshold:real)")},
{"gap-junction-site", make_call<>(make_gap_junction_site,
"'gap-junction-site' with 0 arguments")},
- {"ion-reversal-potential-method", make_call<std::string, arb::mechanism_desc>(make_ion_reversal_potential_method,
- "'ion-reversal-potential-method' with 2 ""arguments (ion:string mech:mechanism)")},
+ {"ion-reversal-potential-method", make_call<std::string, arb::mechanism_desc>(
+ make_ion_reversal_potential_method,
+ "'ion-reversal-potential-method' with 2 arguments (ion:string mech:mechanism)")},
{"mechanism", make_mech_call("'mechanism' with a name argument, and 0 or more parameter settings"
- "(name:string (param:string val:real))")},
-
+ "(name:string (param:string val:real))")},
{"place", make_call<locset, gap_junction_site, std::string>(make_place, "'place' with 3 arguments (ls:locset gj:gap-junction-site name:string)")},
{"place", make_call<locset, i_clamp, std::string>(make_place, "'place' with 3 arguments (ls:locset c:current-clamp name:string)")},
{"place", make_call<locset, threshold_detector, std::string>(make_place, "'place' with 3 arguments (ls:locset t:threshold-detector name:string)")},
diff --git a/python/mechanism.cpp b/python/mechanism.cpp
index e4103ac7..2a27eccb 100644
--- a/python/mechanism.cpp
+++ b/python/mechanism.cpp
@@ -130,7 +130,7 @@ void register_mechanisms(pybind11::module& m) {
struct py_mech_cat_value_iterator {
py_mech_cat_value_iterator(const arb::mechanism_catalogue &cat_, pybind11::object ref_): state{cat_, ref_} { }
mech_cat_iter_state state;
- arb::mechanism_info next() { state.cat[state.next()]; }
+ arb::mechanism_info next() { return state.cat[state.next()]; }
};
pybind11::class_<py_mech_cat_key_iterator>(m, "MechCatKeyIterator")
--
GitLab