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

@@ -32,6 +32,7 @@
 #include <algorithm>
 #include <cctype>
 #include <ctime>
+#include <sstream>
 
 
 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
+	std::string_view lastname;
-	const char *lastname = nullptr;
 	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();
+		std::string_view::size_type number = 0;
-		const char *colon = strchr(tempstr.c_str(), ':');
+		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));
+				util::stream_format(errorbuf, "Switch location '%s' missing switch name!\n", location);
-				lastname = (char *)"UNK";
+				lastname = "UNK";
 			}
-			name.assign(lastname);
+			name = lastname;
 		}
 
 		// if the number is preceded by a '!' it's active high
-		bool invert = false;
+		bool const invert = tempstr[number] == '!';
-		if (*number == '!')
+		if (invert)
-		{
+			++number;
-			invert = true;
-			number++;
-		}
 
 		// now scan the switch number
 		int swnum = -1;
-		if (sscanf(number, "%d", &swnum) != 1)
+		if (sscanf(&tempstr[number], "%d", &swnum) != 1)
-			errorbuf.append(string_format("Switch location '%s' has invalid format!\n", location));
+			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;
+		std::ostringstream errors;
-		m_portlist.append(device, errors);
+		for (device_t &device : iter)
-		if (!errors.empty())
+		{
-			osd_printf_error("Input port errors:\n%s", errors);
+			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),

src/emu/ioport.h

@@ -25,9 +25,11 @@
 #include <cstdint>
 #include <cstring>
 #include <ctime>
+#include <iosfwd>
 #include <initializer_list>
 #include <list>
 #include <memory>
+#include <string_view>
 #include <vector>
 
 
@@ -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_value ()> 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<std::string, std::unique_ptr<ioport_port>>
 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) \

src/emu/validity.cpp

@@ -22,6 +22,7 @@
 #include "unicode.h"
 
 #include <cctype>
+#include <sstream>
 #include <type_traits>
 #include <typeinfo>
 
@@ -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
-
+			std::ostringstream errorbuf;
-		// report any errors during construction
+			portlist.append(device, errorbuf);
-		if (!errorbuf.empty())
+			if (errorbuf.tellp())
-			osd_printf_error("I/O port error during construction:\n%s\n", errorbuf);
+				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)

src/frontend/mame/infoxml.cpp

@@ -36,6 +36,7 @@
 #include <future>
 #include <locale>
 #include <queue>
+#include <sstream>
 #include <type_traits>
 #include <unordered_set>
 #include <utility>
@@ -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);
+		std::ostringstream errors;
-		overall_unemulated |= device.type().unemulated_features();
+		for (device_t &device : iter)
-		overall_imperfect |= device.type().imperfect_features();
+		{
+			portlist.append(device, errors);
+			overall_unemulated |= device.type().unemulated_features();
+			overall_imperfect |= device.type().imperfect_features();
 
-		if (devtypes && device.owner())
+			if (devtypes && device.owner())
-			devtypes->insert(&device.type());
+				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);
+		std::ostringstream errors;
-		overall_unemulated |= dev.type().unemulated_features();
+		for (device_t &dev : device_enumerator(device))
-		overall_imperfect |= dev.type().imperfect_features();
+		{
+			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()));

src/frontend/mame/ui/devopt.cpp

@@ -18,6 +18,7 @@
 #include "util/unicode.h"
 
 #include <locale>
+#include <sstream>
 
 
 namespace ui {
@@ -235,11 +236,13 @@ void menu_device_config::populate_text(std::optional<text_layout> &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)

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