From f8dc6d93ec2fbfab8a24e47e7ffa5e69623804b7 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Thu, 7 Apr 2022 15:13:30 -0600 Subject: [PATCH] constification: clean up final warning This explains why we use const_cast<>. We also fix a logic error that crept in here, in which a source might have its state saved twice --- libs/ardour/ardour/session.h | 2 +- libs/ardour/session_state.cc | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h index b15d238412..c6de9664e7 100644 --- a/libs/ardour/ardour/session.h +++ b/libs/ardour/ardour/session.h @@ -2103,7 +2103,7 @@ private: int set_state (const XMLNode& node, int version); // not idempotent XMLNode& get_template (); - void maybe_copy_midifiles (snapshot_t, boost::shared_ptr src, XMLNode*); + bool maybe_copy_midifile (snapshot_t, boost::shared_ptr src, XMLNode*); /* click track */ typedef std::list Clicks; diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc index 2f5951993c..43d33ea8a9 100644 --- a/libs/ardour/session_state.cc +++ b/libs/ardour/session_state.cc @@ -1314,8 +1314,18 @@ Session::state (bool save_template, snapshot_t snapshot_type, bool only_used_ass } if (snapshot_type != NormalSave && fs->within_session ()) { -#warning CONSTIFICATION maybe fix this - const_cast(this)->maybe_copy_midifiles (snapshot_type, siter->second, child); + /* this copying of MIDI files should not really be occuring within a const scope like + ::state(). However the copying is too intertwined with getting the current state of + the Source, and so we can't move it to before ::state() is called. If we are not + switching to a new snapshot (but are making a snapshot), we want the state from + before the copy. If we are switching to a new snapshot, we want the state from after + the copy. This makes it almost impossible to tease this apart. So for now (April + 2022) we use const_cast. + */ + + if (const_cast(this)->maybe_copy_midifile (snapshot_type, siter->second, child)) { + continue; /* state already added to child */ + } } child->add_child_nocopy (siter->second->get_state()); @@ -1475,8 +1485,8 @@ Session::state (bool save_template, snapshot_t snapshot_type, bool only_used_ass return *node; } -void -Session::maybe_copy_midifiles (snapshot_t snapshot_type, boost::shared_ptr src, XMLNode* child) +bool +Session::maybe_copy_midifile (snapshot_t snapshot_type, boost::shared_ptr src, XMLNode* child) { /* copy MIDI sources to new file * @@ -1495,7 +1505,7 @@ Session::maybe_copy_midifiles (snapshot_t snapshot_type, boost::shared_ptr ms; if ((ms = boost::dynamic_pointer_cast (src)) == 0) { - return; + return false; /* No, it was not a MIDI source */ } const std::string ancestor_name = ms->ancestor_name(); @@ -1547,8 +1557,9 @@ Session::maybe_copy_midifiles (snapshot_t snapshot_type, boost::shared_ptradd_child_nocopy (ms->get_state()); } } -} + return true; /* Yes, it was a MIDI file */ +} XMLNode& Session::get_control_protocol_state () const