From fc41ef0c9a73c96528f1e1551734522aa23f4580 Mon Sep 17 00:00:00 2001 From: Vas Crabb Date: Sat, 5 Oct 2024 20:53:59 +1000 Subject: emu/ioport.cpp: Improved validation of DIP switch locations. * Treat an empty switch name as an error. * Treat a non-positive switch number as an error. * Also allocate fewer temporary strings. --- src/emu/ioport.cpp | 91 ++++++++++++++++++++++------------------- src/emu/ioport.h | 22 +++++----- src/emu/validity.cpp | 14 ++++--- src/frontend/mame/infoxml.cpp | 33 +++++++++------ src/frontend/mame/ui/devopt.cpp | 9 ++-- src/frontend/mame/ui/info.cpp | 2 +- 6 files changed, 98 insertions(+), 73 deletions(-) diff --git a/src/emu/ioport.cpp b/src/emu/ioport.cpp index bf0618cc59c..d05a294d8f0 100644 --- a/src/emu/ioport.cpp +++ b/src/emu/ioport.cpp @@ -32,6 +32,7 @@ #include #include #include +#include namespace { @@ -420,16 +421,13 @@ u8 const inp_header::MAGIC[inp_header::OFFS_BASETIME - inp_header::OFFS_MAGIC] = // to the current list //------------------------------------------------- -void ioport_list::append(device_t &device, std::string &errorbuf) +void ioport_list::append(device_t &device, std::ostream &errorbuf) { // no constructor, no list ioport_constructor constructor = device.input_ports(); - if (constructor == nullptr) + if (!constructor) return; - // reset error buffer - errorbuf.clear(); - // detokenize into the list (*constructor)(device, *this, errorbuf); @@ -692,7 +690,7 @@ ioport_setting::ioport_setting(ioport_field &field, ioport_value _value, const c // ioport_diplocation - constructor //------------------------------------------------- -ioport_diplocation::ioport_diplocation(const char *name, u8 swnum, bool invert) : +ioport_diplocation::ioport_diplocation(std::string_view name, u8 swnum, bool invert) : m_name(name), m_number(swnum), m_invert(invert) @@ -1359,7 +1357,7 @@ float ioport_field::crosshair_read() const // descriptions //------------------------------------------------- -void ioport_field::expand_diplocation(const char *location, std::string &errorbuf) +void ioport_field::expand_diplocation(const char *location, std::ostream &errorbuf) { // if nothing present, bail if (!location) @@ -1368,70 +1366,76 @@ void ioport_field::expand_diplocation(const char *location, std::string &errorbu m_diploclist.clear(); // parse the string - std::string name; // Don't move this variable inside the loop, lastname's lifetime depends on it being outside - const char *lastname = nullptr; + std::string_view lastname; const char *curentry = location; int entries = 0; - while (*curentry != 0) + while (*curentry) { // find the end of this entry const char *comma = strchr(curentry, ','); - if (comma == nullptr) + if (!comma) comma = curentry + strlen(curentry); // extract it to tempbuf - std::string tempstr(curentry, comma - curentry); + std::string_view tempstr(curentry, comma - curentry); // first extract the switch name if present - const char *number = tempstr.c_str(); - const char *colon = strchr(tempstr.c_str(), ':'); + std::string_view::size_type number = 0; + std::string_view::size_type const colon = tempstr.find(':'); - if (colon != nullptr) + std::string_view name; + if (colon != std::string_view::npos) { // allocate and copy the name if it is present - lastname = name.assign(number, colon - number).c_str(); + lastname = tempstr.substr(0, colon); number = colon + 1; + if (lastname.empty()) + { + util::stream_format(errorbuf, "Switch location '%s' has empty switch name!\n", location); + lastname = "UNK"; + } + name = lastname; } else { // otherwise, just copy the last name - if (lastname == nullptr) + if (lastname.empty()) { - errorbuf.append(string_format("Switch location '%s' missing switch name!\n", location)); - lastname = (char *)"UNK"; + util::stream_format(errorbuf, "Switch location '%s' missing switch name!\n", location); + lastname = "UNK"; } - name.assign(lastname); + name = lastname; } // if the number is preceded by a '!' it's active high - bool invert = false; - if (*number == '!') - { - invert = true; - number++; - } + bool const invert = tempstr[number] == '!'; + if (invert) + ++number; // now scan the switch number int swnum = -1; - if (sscanf(number, "%d", &swnum) != 1) - errorbuf.append(string_format("Switch location '%s' has invalid format!\n", location)); + if (sscanf(&tempstr[number], "%d", &swnum) != 1) + util::stream_format(errorbuf, "Switch location '%s' has invalid format!\n", location); + else if (0 >= swnum) + util::stream_format(errorbuf, "Switch location '%s' has switch number that is not positive!\n", location); // allocate a new entry - m_diploclist.emplace_back(name.c_str(), swnum, invert); + if (0 < swnum) + m_diploclist.emplace_back(name, swnum, invert); entries++; // advance to the next item curentry = comma; - if (*curentry != 0) + if (*curentry) curentry++; } // then verify the number of bits in the mask matches int const bits = population_count_32(m_mask); if (bits > entries) - errorbuf.append(string_format("Switch location '%s' does not describe enough bits for mask %X\n", location, m_mask)); + util::stream_format(errorbuf, "Switch location '%s' does not describe enough bits for mask %X\n", location, m_mask); else if (bits < entries) - errorbuf.append(string_format("Switch location '%s' describes too many bits for mask %X\n", location, m_mask)); + util::stream_format(errorbuf, "Switch location '%s' describes too many bits for mask %X\n", location, m_mask); } @@ -1638,7 +1642,7 @@ void ioport_port::frame_update() // wholly overlapped by other fields //------------------------------------------------- -void ioport_port::collapse_fields(std::string &errorbuf) +void ioport_port::collapse_fields(std::ostream &errorbuf) { ioport_value maskbits = 0; int lastmodcount = -1; @@ -1667,13 +1671,13 @@ void ioport_port::collapse_fields(std::string &errorbuf) // for errors //------------------------------------------------- -void ioport_port::insert_field(ioport_field &newfield, ioport_value &disallowedbits, std::string &errorbuf) +void ioport_port::insert_field(ioport_field &newfield, ioport_value &disallowedbits, std::ostream &errorbuf) { // verify against the disallowed bits, but only if we are condition-free if (newfield.condition().none()) { if ((newfield.mask() & disallowedbits) != 0) - errorbuf.append(string_format("INPUT_TOKEN_FIELD specifies duplicate port bits (port=%s mask=%X)\n", tag(), newfield.mask())); + util::stream_format(errorbuf, "INPUT_TOKEN_FIELD specifies duplicate port bits (port=%s mask=%X)\n", tag(), newfield.mask()); disallowedbits |= newfield.mask(); } @@ -1812,12 +1816,17 @@ time_t ioport_manager::initialize() // if we have a token list, proceed device_enumerator iter(machine().root_device()); - for (device_t &device : iter) { - std::string errors; - m_portlist.append(device, errors); - if (!errors.empty()) - osd_printf_error("Input port errors:\n%s", errors); + std::ostringstream errors; + for (device_t &device : iter) + { + m_portlist.append(device, errors); + if (errors.tellp()) + { + osd_printf_error("Input port errors:\n%s", std::move(errors).str()); + errors.str(""); + } + } } // renumber player numbers for controller ports @@ -3274,7 +3283,7 @@ void ioport_manager::record_port(ioport_port &port) // ioport_configurer - constructor //------------------------------------------------- -ioport_configurer::ioport_configurer(device_t &owner, ioport_list &portlist, std::string &errorbuf) : +ioport_configurer::ioport_configurer(device_t &owner, ioport_list &portlist, std::ostream &errorbuf) : m_owner(owner), m_portlist(portlist), m_errorbuf(errorbuf), diff --git a/src/emu/ioport.h b/src/emu/ioport.h index d40ef58ed4a..ff77344fb23 100644 --- a/src/emu/ioport.h +++ b/src/emu/ioport.h @@ -25,9 +25,11 @@ #include #include #include +#include #include #include #include +#include #include @@ -325,7 +327,7 @@ enum //************************************************************************** // constructor function pointer -typedef void(*ioport_constructor)(device_t &owner, ioport_list &portlist, std::string &errorbuf); +typedef void(*ioport_constructor)(device_t &owner, ioport_list &portlist, std::ostream &errorbuf); // I/O port callback function delegates typedef device_delegate ioport_field_read_delegate; @@ -529,7 +531,7 @@ class ioport_diplocation { public: // construction/destruction - ioport_diplocation(const char *name, u8 swnum, bool invert); + ioport_diplocation(std::string_view name, u8 swnum, bool invert); // getters const char *name() const { return m_name.c_str(); } @@ -664,7 +666,7 @@ public: void set_user_settings(const user_settings &settings); private: - void expand_diplocation(const char *location, std::string &errorbuf); + void expand_diplocation(const char *location, std::ostream &errorbuf); // internal state ioport_field * m_next; // pointer to next field in sequence @@ -744,7 +746,7 @@ class ioport_list : public std::map> public: ioport_list() { } - void append(device_t &device, std::string &errorbuf); + void append(device_t &device, std::ostream &errorbuf); }; @@ -779,13 +781,13 @@ public: // other operations ioport_field *field(ioport_value mask) const; - void collapse_fields(std::string &errorbuf); + void collapse_fields(std::ostream &errorbuf); void frame_update(); void init_live_state(); void update_defvalue(bool flush_defaults); private: - void insert_field(ioport_field &newfield, ioport_value &disallowedbits, std::string &errorbuf); + void insert_field(ioport_field &newfield, ioport_value &disallowedbits, std::ostream &errorbuf); // internal state ioport_port * m_next; // pointer to next port @@ -1033,7 +1035,7 @@ class ioport_configurer { public: // construction/destruction - ioport_configurer(device_t &owner, ioport_list &portlist, std::string &errorbuf); + ioport_configurer(device_t &owner, ioport_list &portlist, std::ostream &errorbuf); // static helpers static const char *string_from_token(const char *string); @@ -1083,7 +1085,7 @@ private: // internal state device_t & m_owner; ioport_list & m_portlist; - std::string & m_errorbuf; + std::ostream & m_errorbuf; ioport_port * m_curport; ioport_field * m_curfield; @@ -1123,7 +1125,7 @@ private: // start of table #define INPUT_PORTS_START(_name) \ -ATTR_COLD void INPUT_PORTS_NAME(_name)(device_t &owner, ioport_list &portlist, std::string &errorbuf) \ +ATTR_COLD void INPUT_PORTS_NAME(_name)(device_t &owner, ioport_list &portlist, std::ostream &errorbuf) \ { \ ioport_configurer configurer(owner, portlist, errorbuf); // end of table @@ -1132,7 +1134,7 @@ ATTR_COLD void INPUT_PORTS_NAME(_name)(device_t &owner, ioport_list &portlist, s // aliasing #define INPUT_PORTS_EXTERN(_name) \ - extern void INPUT_PORTS_NAME(_name)(device_t &owner, ioport_list &portlist, std::string &errorbuf) + extern void INPUT_PORTS_NAME(_name)(device_t &owner, ioport_list &portlist, std::ostream &errorbuf) // including #define PORT_INCLUDE(_name) \ diff --git a/src/emu/validity.cpp b/src/emu/validity.cpp index 4cb6202b4d9..f95395fa159 100644 --- a/src/emu/validity.cpp +++ b/src/emu/validity.cpp @@ -22,6 +22,7 @@ #include "unicode.h" #include +#include #include #include @@ -2495,12 +2496,13 @@ void validity_checker::validate_inputs(device_t &root) // allocate the input ports ioport_list portlist; - std::string errorbuf; - portlist.append(device, errorbuf); - - // report any errors during construction - if (!errorbuf.empty()) - osd_printf_error("I/O port error during construction:\n%s\n", errorbuf); + { + // report any errors during construction + std::ostringstream errorbuf; + portlist.append(device, errorbuf); + if (errorbuf.tellp()) + osd_printf_error("I/O port error during construction:\n%s\n", std::move(errorbuf).str()); + } // do a first pass over ports to add their names and find duplicates for (auto &port : portlist) diff --git a/src/frontend/mame/infoxml.cpp b/src/frontend/mame/infoxml.cpp index 37d649a1131..95ca0826ede 100644 --- a/src/frontend/mame/infoxml.cpp +++ b/src/frontend/mame/infoxml.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -696,17 +697,19 @@ void output_one(std::ostream &out, driver_enumerator &drivlist, const game_drive // allocate input ports and build overall emulation status ioport_list portlist; - std::string errors; device_t::feature_type overall_unemulated(driver.type.unemulated_features()); device_t::feature_type overall_imperfect(driver.type.imperfect_features()); - for (device_t &device : iter) { - portlist.append(device, errors); - overall_unemulated |= device.type().unemulated_features(); - overall_imperfect |= device.type().imperfect_features(); + std::ostringstream errors; + for (device_t &device : iter) + { + portlist.append(device, errors); + overall_unemulated |= device.type().unemulated_features(); + overall_imperfect |= device.type().imperfect_features(); - if (devtypes && device.owner()) - devtypes->insert(&device.type()); + if (devtypes && device.owner()) + devtypes->insert(&device.type()); + } } // renumber player numbers for controller ports @@ -815,24 +818,30 @@ void output_one_device(std::ostream &out, machine_config &config, device_t &devi // generate input list and build overall emulation status ioport_list portlist; - std::string errors; device_t::feature_type overall_unemulated(device.type().unemulated_features()); device_t::feature_type overall_imperfect(device.type().imperfect_features()); - for (device_t &dev : device_enumerator(device)) { - portlist.append(dev, errors); - overall_unemulated |= dev.type().unemulated_features(); - overall_imperfect |= dev.type().imperfect_features(); + std::ostringstream errors; + for (device_t &dev : device_enumerator(device)) + { + portlist.append(dev, errors); + overall_unemulated |= dev.type().unemulated_features(); + overall_imperfect |= dev.type().imperfect_features(); + } } // check if the device adds player inputs (other than dsw and configs) to the system for (auto &port : portlist) + { for (ioport_field const &field : port.second->fields()) + { if (field.type() >= IPT_START1 && field.type() < IPT_UI_FIRST) { has_input = true; break; } + } + } // start to output info util::stream_format(out, "\t<%s name=\"%s\"", XML_TOP, normalize_string(device.shortname())); diff --git a/src/frontend/mame/ui/devopt.cpp b/src/frontend/mame/ui/devopt.cpp index 8d841c7d131..40f60d9b59f 100644 --- a/src/frontend/mame/ui/devopt.cpp +++ b/src/frontend/mame/ui/devopt.cpp @@ -18,6 +18,7 @@ #include "util/unicode.h" #include +#include namespace ui { @@ -235,11 +236,13 @@ void menu_device_config::populate_text(std::optional &layout, float int input = 0, input_mj = 0, input_hana = 0, input_gamble = 0, input_analog = 0, input_adjust = 0; int input_keypad = 0, input_keyboard = 0, dips = 0, confs = 0; - std::string errors; std::ostringstream dips_opt, confs_opt; ioport_list portlist; - for (device_t &iptdev : device_enumerator(*dev)) - portlist.append(iptdev, errors); + { + std::ostringstream errors; + for (device_t &iptdev : device_enumerator(*dev)) + portlist.append(iptdev, errors); + } // check if the device adds inputs to the system for (auto &port : portlist) diff --git a/src/frontend/mame/ui/info.cpp b/src/frontend/mame/ui/info.cpp index 945c18d32a2..c2ba4af2cfb 100644 --- a/src/frontend/mame/ui/info.cpp +++ b/src/frontend/mame/ui/info.cpp @@ -230,7 +230,7 @@ machine_static_info::machine_static_info(const ui_options &options, machine_conf , m_has_analog(false) { ioport_list local_ports; - std::string sink; + std::ostringstream sink; for (device_t &device : device_enumerator(config.root_device())) { // the "no sound hardware" warning doesn't make sense when you plug in a sound card -- cgit v1.2.3