diff options
author | David Robillard <d@drobilla.net> | 2013-01-20 07:33:27 +0000 |
---|---|---|
committer | David Robillard <d@drobilla.net> | 2013-01-20 07:33:27 +0000 |
commit | 64f1a8e8936c97ca4c55a7a4dc97668f19d7d26f (patch) | |
tree | 2aa7ed377c4de6347e0418a500bc7e0d562e0a82 /libs/midi++2/midnam_patch.cc | |
parent | 022e590429acfb3e3361734035aa1543527e9bc4 (diff) |
Error checking string to int conversion from midnam files.
Use const references where appropriate.
Fix incorrect use of (either confusingly named or often abused) Patch tag Number attribute.
git-svn-id: svn://localhost/ardour2/branches/3.0@13923 d708f5d6-7413-0410-9779-e7cbd77b26cf
Diffstat (limited to 'libs/midi++2/midnam_patch.cc')
-rw-r--r-- | libs/midi++2/midnam_patch.cc | 95 |
1 files changed, 50 insertions, 45 deletions
diff --git a/libs/midi++2/midnam_patch.cc b/libs/midi++2/midnam_patch.cc index 916dfe7be7..bb9e27ebff 100644 --- a/libs/midi++2/midnam_patch.cc +++ b/libs/midi++2/midnam_patch.cc @@ -14,10 +14,10 @@ You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - - $Id$ */ +#include <stdlib.h> + #include <algorithm> #include <iostream> @@ -42,7 +42,21 @@ Patch::Patch (std::string name, uint8_t p_number, uint16_t b_number) { } -int initialize_primary_key_from_commands (PatchPrimaryKey& id, const XMLNode* node) +static int +string_to_int(const XMLTree& tree, const std::string& str) +{ + char* endptr = NULL; + const int i = strtol(str.c_str(), &endptr, 10); + if (str.empty() || *endptr != '\0') { + PBD::error << string_compose("%1: Bad number `%2'", tree.filename(), str) + << endmsg; + } + return i; +} + +static int +initialize_primary_key_from_commands ( + const XMLTree& tree, PatchPrimaryKey& id, const XMLNode* node) { id.bank_number = 0; @@ -51,21 +65,19 @@ int initialize_primary_key_from_commands (PatchPrimaryKey& id, const XMLNode* no XMLNode* node = *i; if (node->name() == "ControlChange") { - string control = node->property("Control")->value(); - assert(control != ""); - string value = node->property("Value")->value(); - assert(value != ""); + const string& control = node->property("Control")->value(); + const string& value = node->property("Value")->value(); if (control == "0") { - id.bank_number |= (PBD::atoi (value)) << 7; + id.bank_number |= string_to_int(tree, value) << 7; } else if (control == "32") { - id.bank_number |= PBD::atoi (value); + id.bank_number |= string_to_int(tree, value); } } else if (node->name() == "ProgramChange") { - string number = node->property("Number")->value(); + const string& number = node->property("Number")->value(); assert(number != ""); - id.program_number = PBD::atoi(number); + id.program_number = string_to_int(tree, number); } } @@ -96,37 +108,33 @@ Patch::get_state (void) } int -Patch::set_state (const XMLTree&, const XMLNode& node) +Patch::set_state (const XMLTree& tree, const XMLNode& node) { if (node.name() != "Patch") { cerr << "Incorrect node " << node.name() << " handed to Patch" << endl; return -1; } - const XMLProperty* prop = node.property ("Number"); + /* Note there is a "Number" attribute, but it's really more like a label + and is often not numeric. We currently do not use it. */ - if (!prop) { - return -1; + const XMLProperty* program_change = node.property("ProgramChange"); + if (program_change) { + _id.program_number = string_to_int(tree, program_change->value()); } - _id.program_number = PBD::atoi (prop->value()); - - prop = node.property ("Name"); - if (!prop) { + const XMLProperty* name = node.property("Name"); + if (!name) { return -1; } - _name = prop->value(); + _name = name->value(); XMLNode* commands = node.child("PatchMIDICommands"); - if (commands) { - if (initialize_primary_key_from_commands(_id, commands)) { - return -1; + if (initialize_primary_key_from_commands(tree, _id, commands) && + !program_change) { + return -1; // Failed to find a program number anywhere } - } else { - string program_change = node.property("ProgramChange")->value(); - assert(program_change.length()); - _id.program_number = PBD::atoi(program_change); } XMLNode* use_note_name_list = node.child("UsesNoteNameList"); @@ -152,21 +160,17 @@ Note::set_state (const XMLTree& tree, const XMLNode& node) { assert(node.name() == "Note"); - /* If the note number is junk, this will pull a number from the start, or - return zero if there isn't one. The decrement will make that zero - illegal since note numbers in the file are one-based. Better error - detection would be a good idea, but the duplicate check in - NoteNameList::set_state() will probably catch most errors anyway. */ - _number = atoi(node.property("Number")->value().c_str()) - 1; - _name = node.property("Name")->value(); - - if (_number > 127) { + const int num = string_to_int(tree, node.property("Number")->value()); + if (num < 1 || num > 128) { PBD::warning << string_compose("%1: Note number %2 (%3) out of range", - tree.filename(), (int)_number, _name) + tree.filename(), num, _name) << endmsg; return -1; } + _number = num - 1; + _name = node.property("Name")->value(); + return 0; } @@ -297,7 +301,7 @@ PatchBank::set_state (const XMLTree& tree, const XMLNode& node) XMLNode* commands = node.child("MIDICommands"); if (commands) { PatchPrimaryKey id (0, 0); - if (initialize_primary_key_from_commands (id, commands)) { + if (initialize_primary_key_from_commands (tree, id, commands)) { return -1; } _number = id.bank_number; @@ -439,7 +443,8 @@ ChannelNameSet::set_state (const XMLTree& tree, const XMLNode& node) for(XMLSharedNodeList::const_iterator i = channels->begin(); i != channels->end(); ++i) { - _available_for_channels.insert(atoi((*i)->attribute_value().c_str())); + _available_for_channels.insert( + string_to_int(tree, (*i)->attribute_value())); } } @@ -476,8 +481,8 @@ CustomDeviceMode::set_state(const XMLTree& tree, const XMLNode& a_node) for(XMLSharedNodeList::const_iterator i = channel_name_set_assignments->begin(); i != channel_name_set_assignments->end(); ++i) { - int channel = atoi((*i)->property("Channel")->value().c_str()); - string name_set = (*i)->property("NameSet")->value(); + const int channel = string_to_int(tree, (*i)->property("Channel")->value()); + const string& name_set = (*i)->property("NameSet")->value(); assert( 1 <= channel && channel <= 16 ); _channel_name_set_assignments[channel - 1] = name_set; } @@ -502,13 +507,13 @@ CustomDeviceMode::get_state(void) } boost::shared_ptr<CustomDeviceMode> -MasterDeviceNames::custom_device_mode_by_name(std::string mode_name) +MasterDeviceNames::custom_device_mode_by_name(const std::string& mode_name) { return _custom_device_modes[mode_name]; } boost::shared_ptr<ChannelNameSet> -MasterDeviceNames::channel_name_set_by_device_mode_and_channel(std::string mode, uint8_t channel) +MasterDeviceNames::channel_name_set_by_device_mode_and_channel(const std::string& mode, uint8_t channel) { boost::shared_ptr<CustomDeviceMode> cdm = custom_device_mode_by_name(mode); boost::shared_ptr<ChannelNameSet> cns = _channel_name_sets[cdm->channel_name_set_name_by_channel(channel)]; @@ -516,7 +521,7 @@ MasterDeviceNames::channel_name_set_by_device_mode_and_channel(std::string mode, } boost::shared_ptr<Patch> -MasterDeviceNames::find_patch(std::string mode, uint8_t channel, const PatchPrimaryKey& key) +MasterDeviceNames::find_patch(const std::string& mode, uint8_t channel, const PatchPrimaryKey& key) { return channel_name_set_by_device_mode_and_channel(mode, channel)->find_patch(key); } @@ -668,7 +673,7 @@ MasterDeviceNames::set_state(const XMLTree& tree, const XMLNode&) PatchNameLists::iterator p; for (ChannelNameSet::PatchBanks::iterator pb = pbs.begin(); pb != pbs.end(); ++pb) { - std::string pln = (*pb)->patch_list_name(); + const std::string& pln = (*pb)->patch_list_name(); if (!pln.empty()) { if ((p = _patch_name_lists.find (pln)) != _patch_name_lists.end()) { if ((*pb)->set_patch_name_list (p->second)) { |