From a9ca7f649a1ff84409ef7e86166b1161e8848efb Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 18 Mar 2017 21:31:34 +0100 Subject: Resolve copy-c'tor and assignment issue with TempoMap There are various issues with copy-construction: no readlock is taken, Tempo/Metric Sections were static-cast to non-const pointers and passed as references... This remove the [now] unused copy-c'tor, and fixes various const issues. --- libs/ardour/ardour/tempo.h | 7 ++++--- libs/ardour/tempo.cc | 46 ++++++++++++---------------------------------- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/libs/ardour/ardour/tempo.h b/libs/ardour/ardour/tempo.h index dcf68d7f16..0da03925c0 100644 --- a/libs/ardour/ardour/tempo.h +++ b/libs/ardour/ardour/tempo.h @@ -323,7 +323,6 @@ class LIBARDOUR_API TempoMap : public PBD::StatefulDestructible { public: TempoMap (framecnt_t frame_rate); - TempoMap (TempoMap const &); ~TempoMap(); TempoMap& operator= (TempoMap const &); @@ -519,6 +518,8 @@ class LIBARDOUR_API TempoMap : public PBD::StatefulDestructible void fix_legacy_end_session(); private: + /* prevent copy construction */ + TempoMap (TempoMap const&); TempoSection* previous_tempo_section_locked (const Metrics& metrics, TempoSection*) const; TempoSection* next_tempo_section_locked (const Metrics& metrics, TempoSection*) const; @@ -604,8 +605,8 @@ private: bool remove_tempo_locked (const TempoSection&); bool remove_meter_locked (const MeterSection&); - TempoSection* copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSection* section); - MeterSection* copy_metrics_and_point (const Metrics& metrics, Metrics& copy, MeterSection* section); + TempoSection* copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSection* section) const; + MeterSection* copy_metrics_and_point (const Metrics& metrics, Metrics& copy, MeterSection* section) const; }; }; /* namespace ARDOUR */ diff --git a/libs/ardour/tempo.cc b/libs/ardour/tempo.cc index cc40ef5532..45ae54f479 100644 --- a/libs/ardour/tempo.cc +++ b/libs/ardour/tempo.cc @@ -784,27 +784,12 @@ TempoMap::TempoMap (framecnt_t fr) } -TempoMap::TempoMap (TempoMap const & other) -{ - _frame_rate = other._frame_rate; - for (Metrics::const_iterator m = other._metrics.begin(); m != other._metrics.end(); ++m) { - TempoSection* ts = dynamic_cast (*m); - MeterSection* ms = dynamic_cast (*m); - - if (ts) { - TempoSection* new_section = new TempoSection (*ts); - _metrics.push_back (new_section); - } else { - MeterSection* new_section = new MeterSection (*ms); - _metrics.push_back (new_section); - } - } -} - TempoMap& TempoMap::operator= (TempoMap const & other) { if (&other != this) { + Glib::Threads::RWLock::ReaderLock lr (other.lock); + Glib::Threads::RWLock::WriterLock lm (lock); _frame_rate = other._frame_rate; Metrics::const_iterator d = _metrics.begin(); @@ -815,8 +800,8 @@ TempoMap::operator= (TempoMap const & other) _metrics.clear(); for (Metrics::const_iterator m = other._metrics.begin(); m != other._metrics.end(); ++m) { - TempoSection* ts = dynamic_cast (*m); - MeterSection* ms = dynamic_cast (*m); + TempoSection const * const ts = dynamic_cast (*m); + MeterSection const * const ms = dynamic_cast (*m); if (ts) { TempoSection* new_section = new TempoSection (*ts); @@ -3140,15 +3125,13 @@ TempoMap::solve_map_bbt (Metrics& imaginary, MeterSection* section, const BBT_Ti * to section's equivalent in copy. */ TempoSection* -TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSection* section) +TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSection* section) const { TempoSection* ret = 0; for (Metrics::const_iterator i = metrics.begin(); i != metrics.end(); ++i) { - TempoSection* t; - MeterSection* m; if ((*i)->is_tempo()) { - t = static_cast (*i); + TempoSection const * const t = dynamic_cast (*i); if (t == section) { ret = new TempoSection (*t); copy.push_back (ret); @@ -3157,9 +3140,8 @@ TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSe TempoSection* cp = new TempoSection (*t); copy.push_back (cp); - } - if (!(*i)->is_tempo()) { - m = static_cast (*i); + } else { + MeterSection const * const m = dynamic_cast (*i); MeterSection* cp = new MeterSection (*m); copy.push_back (cp); } @@ -3169,21 +3151,17 @@ TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, TempoSe } MeterSection* -TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, MeterSection* section) +TempoMap::copy_metrics_and_point (const Metrics& metrics, Metrics& copy, MeterSection* section) const { MeterSection* ret = 0; for (Metrics::const_iterator i = metrics.begin(); i != metrics.end(); ++i) { - TempoSection* t; - MeterSection* m; if ((*i)->is_tempo()) { - t = static_cast (*i); + TempoSection const * const t = dynamic_cast (*i); TempoSection* cp = new TempoSection (*t); copy.push_back (cp); - } - - if (!(*i)->is_tempo()) { - m = static_cast (*i); + } else { + MeterSection const * const m = dynamic_cast (*i); if (m == section) { ret = new MeterSection (*m); copy.push_back (ret); -- cgit v1.2.3