From 699ac222ff6d1ab4451e35391192a16bc53a61bf Mon Sep 17 00:00:00 2001 From: Matthieu Dorier Date: Wed, 13 Feb 2019 08:28:26 -0600 Subject: [PATCH] more safety in DataSet and Run classes --- src/DataSet.cpp | 20 ++++++++++---- src/DataStore.cpp | 17 +++++++++--- src/Run.cpp | 65 ++++++++++++++++++++++++++++++++++++-------- test/DataSetTest.cpp | 2 +- test/RunSetTest.cpp | 2 +- 5 files changed, 84 insertions(+), 22 deletions(-) diff --git a/src/DataSet.cpp b/src/DataSet.cpp index 22779fc..476e00a 100644 --- a/src/DataSet.cpp +++ b/src/DataSet.cpp @@ -30,12 +30,15 @@ DataSet::DataSet(DataStore* ds, uint8_t level, const std::string& fullname) DataSet::DataSet(DataStore* ds, uint8_t level, const std::string& container, const std::string& name) : m_impl(std::make_unique(this, ds, level, container, name)) {} -DataSet::DataSet(const DataSet& other) -: m_impl(std::make_unique( +DataSet::DataSet(const DataSet& other) { + if(other.m_impl) { + m_impl = std::make_unique( this, other.m_impl->m_datastore, other.m_impl->m_level, other.m_impl->m_container, - other.m_impl->m_name)) {} + other.m_impl->m_name); + } +} DataSet::DataSet(DataSet&& other) : m_impl(std::move(other.m_impl)) { @@ -46,6 +49,10 @@ DataSet::DataSet(DataSet&& other) DataSet& DataSet::operator=(const DataSet& other) { if(this == &other) return *this; + if(!other.m_impl) { + m_impl.reset(); + return *this; + } m_impl = std::make_unique(this, other.m_impl->m_datastore, other.m_impl->m_level, @@ -103,8 +110,11 @@ bool DataSet::loadRawData(const std::string& key, std::vector& buffer) con } bool DataSet::operator==(const DataSet& other) const { - if(!valid() && !other.valid()) - return true; + bool v1 = valid(); + bool v2 = other.valid(); + if(!v1 && !v2) return true; + if(v1 && !v2) return false; + if(!v2 && v2) return false; return m_impl->m_datastore == other.m_impl->m_datastore && m_impl->m_level == other.m_impl->m_level && m_impl->m_container == other.m_impl->m_container diff --git a/src/DataStore.cpp b/src/DataStore.cpp index 91a9ab6..6741b79 100644 --- a/src/DataStore.cpp +++ b/src/DataStore.cpp @@ -240,15 +240,21 @@ DataStore::const_iterator::const_iterator(DataSet&& dataset) DataStore::const_iterator::~const_iterator() {} -DataStore::const_iterator::const_iterator(const DataStore::const_iterator& other) -: m_impl(std::make_unique(*other.m_impl)) {} +DataStore::const_iterator::const_iterator(const DataStore::const_iterator& other) { + if(other.m_impl) { + m_impl = std::make_unique(*other.m_impl); + } +} DataStore::const_iterator::const_iterator(DataStore::const_iterator&& other) : m_impl(std::move(other.m_impl)) {} DataStore::const_iterator& DataStore::const_iterator::operator=(const DataStore::const_iterator& other) { if(&other == this) return *this; - m_impl = std::make_unique(*other.m_impl); + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); + else + m_impl.reset(); return *this; } @@ -320,7 +326,10 @@ DataStore::iterator::iterator(DataStore::iterator&& other) DataStore::iterator& DataStore::iterator::operator=(const DataStore::iterator& other) { if(this == &other) return *this; - m_impl = std::make_unique(*other.m_impl); + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); + else + m_impl.reset(); return *this; } diff --git a/src/Run.cpp b/src/Run.cpp index 3c3615c..642f350 100644 --- a/src/Run.cpp +++ b/src/Run.cpp @@ -16,14 +16,18 @@ Run::Run() Run::Run(DataStore* ds, uint8_t level, const std::string& container, const RunNumber& rn) : m_impl(std::make_unique(ds, level, container, rn)) { } -Run::Run(const Run& other) -: m_impl(std::make_unique(*other.m_impl)) {} +Run::Run(const Run& other) { + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); +} Run::Run(Run&&) = default; Run& Run::operator=(const Run& other) { if(this == &other) return *this; - m_impl = std::make_unique(*other.m_impl); + if(other.m_impl) { + m_impl = std::make_unique(*other.m_impl); + } return *this; } @@ -32,7 +36,9 @@ Run& Run::operator=(Run&&) = default; Run::~Run() = default; DataStore* Run::getDataStore() const { - if(!m_impl) return nullptr; + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_datastore; } @@ -62,7 +68,7 @@ bool Run::valid() const { ProductID Run::storeRawData(const std::string& key, const std::vector& buffer) { if(!valid()) { - throw Exception("Calling store() on invalid Run"); + throw Exception("Calling Run member function on an invalid Run object"); } // forward the call to the datastore's store function return m_impl->m_datastore->m_impl->store(0, m_impl->fullpath(), key, buffer); @@ -70,13 +76,18 @@ ProductID Run::storeRawData(const std::string& key, const std::vector& buf bool Run::loadRawData(const std::string& key, std::vector& buffer) const { if(!valid()) { - throw Exception("Calling load() on invalid Run"); + throw Exception("Calling Run member function on an invalid Run object"); } // forward the call to the datastore's load function return m_impl->m_datastore->m_impl->load(0, m_impl->fullpath(), key, buffer); } bool Run::operator==(const Run& other) const { + bool v1 = valid(); + bool v2 = other.valid(); + if(!v1 && !v2) return true; + if(v1 && !v2) return false; + if(!v1 && v2) return false; return m_impl->m_datastore == other.m_impl->m_datastore && m_impl->m_level == other.m_impl->m_level && m_impl->m_container == other.m_impl->m_container @@ -88,14 +99,23 @@ bool Run::operator!=(const Run& other) const { } const RunNumber& Run::number() const { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_run_nr; } const std::string& Run::container() const { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_container; } SubRun Run::createSubRun(const SubRunNumber& subRunNumber) { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } std::string parent = m_impl->fullpath(); std::string subRunStr = SubRun::Impl::makeKeyStringFromSubRunNumber(subRunNumber); m_impl->m_datastore->m_impl->store(m_impl->m_level+1, parent, subRunStr, std::vector()); @@ -108,6 +128,9 @@ SubRun Run::operator[](const SubRunNumber& subRunNumber) const { } Run::iterator Run::find(const SubRunNumber& subRunNumber) { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } int ret; std::vector data; std::string parent = m_impl->fullpath(); @@ -139,6 +162,9 @@ Run::iterator Run::begin() { } Run::iterator Run::end() { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_end; } @@ -147,6 +173,9 @@ Run::const_iterator Run::begin() const { } Run::const_iterator Run::end() const { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_end; } @@ -155,6 +184,9 @@ Run::const_iterator Run::cbegin() const { } Run::const_iterator Run::cend() const { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } return m_impl->m_end; } @@ -192,6 +224,9 @@ Run::const_iterator Run::lower_bound(const SubRunNumber& lb) const { } Run::iterator Run::upper_bound(const SubRunNumber& ub) { + if(!valid()) { + throw Exception("Calling Run member function on an invalid Run object"); + } SubRun subrun(m_impl->m_datastore, m_impl->m_level+1, m_impl->fullpath(), ub); @@ -249,15 +284,20 @@ Run::const_iterator::const_iterator(SubRun&& subrun) Run::const_iterator::~const_iterator() {} -Run::const_iterator::const_iterator(const Run::const_iterator& other) -: m_impl(std::make_unique(*other.m_impl)) {} +Run::const_iterator::const_iterator(const Run::const_iterator& other) { + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); +} Run::const_iterator::const_iterator(Run::const_iterator&& other) : m_impl(std::move(other.m_impl)) {} Run::const_iterator& Run::const_iterator::operator=(const Run::const_iterator& other) { if(&other == this) return *this; - m_impl = std::make_unique(*other.m_impl); + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); + else + m_impl.reset(); return *this; } @@ -301,7 +341,7 @@ bool Run::const_iterator::operator==(const self_type& rhs) const { } bool Run::const_iterator::operator!=(const self_type& rhs) const { - return !(*this == rhs); + return !(*this == rhs); } //////////////////////////////////////////////////////////////////////////////////////////// @@ -327,7 +367,10 @@ Run::iterator::iterator(Run::iterator&& other) Run::iterator& Run::iterator::operator=(const Run::iterator& other) { if(this == &other) return *this; - m_impl = std::make_unique(*other.m_impl); + if(other.m_impl) + m_impl = std::make_unique(*other.m_impl); + else + m_impl.reset(); return *this; } diff --git a/test/DataSetTest.cpp b/test/DataSetTest.cpp index 049bd69..a8451dc 100644 --- a/test/DataSetTest.cpp +++ b/test/DataSetTest.cpp @@ -171,7 +171,7 @@ void DataSetTest::testCreateRuns() { { Run r = mds[45]; CPPUNIT_ASSERT(!r.valid()); - CPPUNIT_ASSERT(r.number() == InvalidRunNumber); + CPPUNIT_ASSERT_THROW(r.number(), hepnos::Exception); } { diff --git a/test/RunSetTest.cpp b/test/RunSetTest.cpp index 8b14502..9bd85ad 100644 --- a/test/RunSetTest.cpp +++ b/test/RunSetTest.cpp @@ -20,7 +20,7 @@ void RunSetTest::testFillDataStore() { // default-constructed run has InvalidRunNumber number Run r0; CPPUNIT_ASSERT(!r0.valid()); - CPPUNIT_ASSERT(InvalidRunNumber == r0.number()); + CPPUNIT_ASSERT_THROW(r0.number(), hepnos::Exception); // correct run creation Run r1 = mds.createRun(42); // assert the characteristics of the created dataset -- 2.26.2