From b7a9f3c6b56c5945028ad868fee36fd9f50e9944 Mon Sep 17 00:00:00 2001 From: Tim Mayberry Date: Wed, 19 Apr 2017 20:48:47 +1000 Subject: Use XMLNode::get_property in Session::memento_command_factory Avoids potential issues with dereferencing a NULL XMLProperty pointer and improves readability by using better locally scoped variable names. --- libs/ardour/session_command.cc | 44 ++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 21 deletions(-) (limited to 'libs/ardour/session_command.cc') diff --git a/libs/ardour/session_command.cc b/libs/ardour/session_command.cc index a99738a613..18f282f569 100644 --- a/libs/ardour/session_command.cc +++ b/libs/ardour/session_command.cc @@ -37,6 +37,7 @@ #include "pbd/memento_command.h" #include "pbd/stateful_diff_command.h" #include "pbd/statefuldestructible.h" +#include "pbd/types_convert.h" class Command; @@ -57,13 +58,8 @@ Session::memento_command_factory(XMLNode *n) XMLNode *before = 0, *after = 0; XMLNode *child = 0; - /* get id */ - /* XXX: HACK! */ - bool have_id = n->property("obj-id") != 0; - if (have_id) { - id = PBD::ID(n->property("obj-id")->value()); - } + bool have_id = n->get_property ("obj-id", id); /* get before/after */ @@ -89,43 +85,44 @@ Session::memento_command_factory(XMLNode *n) } /* create command */ - std::string obj_T = n->property ("type-name")->value(); + std::string type_name; + n->get_property ("type-name", type_name); - if (obj_T == "ARDOUR::AudioRegion" || obj_T == "ARDOUR::MidiRegion" || obj_T == "ARDOUR::Region") { + if (type_name == "ARDOUR::AudioRegion" || type_name == "ARDOUR::MidiRegion" || type_name == "ARDOUR::Region") { boost::shared_ptr r = RegionFactory::region_by_id (id); if (r) { return new MementoCommand(*r, before, after); } - } else if (obj_T == "ARDOUR::AudioSource" || obj_T == "ARDOUR::MidiSource") { + } else if (type_name == "ARDOUR::AudioSource" || type_name == "ARDOUR::MidiSource") { if (sources.count(id)) return new MementoCommand(*sources[id], before, after); - } else if (obj_T == "ARDOUR::Location") { + } else if (type_name == "ARDOUR::Location") { Location* loc = _locations->get_location_by_id(id); if (loc) { return new MementoCommand(*loc, before, after); } - } else if (obj_T == "ARDOUR::Locations") { + } else if (type_name == "ARDOUR::Locations") { return new MementoCommand(*_locations, before, after); - } else if (obj_T == "ARDOUR::TempoMap") { + } else if (type_name == "ARDOUR::TempoMap") { return new MementoCommand(*_tempo_map, before, after); - } else if (obj_T == "ARDOUR::Playlist" || obj_T == "ARDOUR::AudioPlaylist" || obj_T == "ARDOUR::MidiPlaylist") { + } else if (type_name == "ARDOUR::Playlist" || type_name == "ARDOUR::AudioPlaylist" || type_name == "ARDOUR::MidiPlaylist") { if (boost::shared_ptr pl = playlists->by_name(child->property("name")->value())) { return new MementoCommand(*(pl.get()), before, after); } - } else if (obj_T == "ARDOUR::Route" || obj_T == "ARDOUR::AudioTrack" || obj_T == "ARDOUR::MidiTrack") { + } else if (type_name == "ARDOUR::Route" || type_name == "ARDOUR::AudioTrack" || type_name == "ARDOUR::MidiTrack") { if (boost::shared_ptr r = route_by_id(id)) { return new MementoCommand(*r, before, after); } else { error << string_compose (X_("Route %1 not found in session"), id) << endmsg; } - } else if (obj_T == "Evoral::Curve" || obj_T == "ARDOUR::AutomationList") { + } else if (type_name == "Evoral::Curve" || type_name == "ARDOUR::AutomationList") { if (have_id) { std::map::iterator i = automation_lists.find(id); if (i != automation_lists.end()) { @@ -145,7 +142,7 @@ Session::memento_command_factory(XMLNode *n) } /* we failed */ - info << string_compose (_("could not reconstitute MementoCommand from XMLNode. object type = %1 id = %2"), obj_T, id.to_s()) << endmsg; + info << string_compose (_("could not reconstitute MementoCommand from XMLNode. object type = %1 id = %2"), type_name, id.to_s()) << endmsg; return 0 ; } @@ -153,16 +150,21 @@ Session::memento_command_factory(XMLNode *n) Command * Session::stateful_diff_command_factory (XMLNode* n) { - PBD::ID const id (n->property("obj-id")->value ()); + PBD::ID id; + std::string type_name; + if (!n->get_property ("obj-id", id) || !n->get_property ("type-name", type_name)) { + error << _("Could get object ID and type name for StatefulDiffCommand from XMLNode.") + << endmsg; + return 0; + } - std::string const obj_T = n->property ("type-name")->value (); - if ((obj_T == "ARDOUR::AudioRegion" || obj_T == "ARDOUR::MidiRegion")) { + if ((type_name == "ARDOUR::AudioRegion" || type_name == "ARDOUR::MidiRegion")) { boost::shared_ptr r = RegionFactory::region_by_id (id); if (r) { return new StatefulDiffCommand (r, *n); } - } else if (obj_T == "ARDOUR::AudioPlaylist" || obj_T == "ARDOUR::MidiPlaylist") { + } else if (type_name == "ARDOUR::AudioPlaylist" || type_name == "ARDOUR::MidiPlaylist") { boost::shared_ptr p = playlists->by_id (id); if (p) { return new StatefulDiffCommand (p, *n); @@ -174,7 +176,7 @@ Session::stateful_diff_command_factory (XMLNode* n) /* we failed */ error << string_compose ( - _("could not reconstitute StatefulDiffCommand from XMLNode. object type = %1 id = %2"), obj_T, id.to_s()) + _("could not reconstitute StatefulDiffCommand from XMLNode. object type = %1 id = %2"), type_name, id.to_s()) << endmsg; return 0; -- cgit v1.2.3