emu/romload.cpp: Tidy up some of the code a little.

src/emu/romload.cpp

@@ -20,6 +20,7 @@
 #include "ui/uimain.h"
 
 #include "corestr.h"
+#include "path.h"
 
 #include <algorithm>
 #include <cstdarg>
@@ -42,6 +43,25 @@
 
 namespace {
 
+/*-------------------------------------------------
+    debugload - log data to a file
+-------------------------------------------------*/
+
+void ATTR_PRINTF(1,2) debugload(const char *string, ...)
+{
+	static int opened = 0;
+	FILE *const f(fopen("romload.log", opened++ ? "a" : "w"));
+	if (f)
+	{
+		va_list arg;
+		va_start(arg, string);
+		vfprintf(f, string, arg);
+		va_end(arg);
+		fclose(f);
+	}
+}
+
+
 auto next_parent_system(game_driver const &system)
 {
 	return
@@ -122,7 +142,7 @@ std::vector<std::string> make_software_searchpath(software_list_device &swlist,
 		if (std::find(parents.begin(), parents.end(), i) != parents.end())
 			break;
 		parents.emplace_back(i);
-		result.emplace_back(util::string_format("%s" PATH_SEPARATOR "%s", swlist.list_name(), i->shortname()));
+		result.emplace_back(util::path_concat(swlist.list_name(), i->shortname()));
 		i = i->parentname().empty() ? nullptr : swlist.find(i->parentname());
 	}
 
@@ -144,9 +164,9 @@ std::error_condition do_open_disk(
 {
 	// hashes are fixed, but we might need to try multiple filenames
 	std::set<std::string> tried;
-	const util::hash_collection hashes(romp->hashdata());
+	util::hash_collection const hashes(romp->hashdata());
 	std::string filename, fullpath;
-	const rom_entry *parent(nullptr);
+	rom_entry const *parent(nullptr);
 	std::error_condition result(std::errc::no_such_file_or_directory);
 	while (romp && result)
 	{
@@ -155,11 +175,11 @@ std::error_condition do_open_disk(
 		{
 			// piggyback on emu_file to find the disk image file
 			std::unique_ptr<emu_file> imgfile;
-			for (const std::vector<std::string> &paths : searchpath)
+			for (std::vector<std::string> const &paths : searchpath)
 			{
 				imgfile.reset(new emu_file(options.media_path(), paths, OPEN_FLAG_READ));
 				imgfile->set_restrict_to_mediapath(1);
-				const std::error_condition filerr(imgfile->open(filename, OPEN_FLAG_READ));
+				std::error_condition const filerr(imgfile->open(filename, OPEN_FLAG_READ));
 				if (!filerr)
 					break;
 				else
@@ -225,7 +245,7 @@ auto open_parent_disk(
 			{
 				util::hash_collection hashes;
 				hashes.add_sha1(sha1);
-				std::function<rom_entry const * ()> np(next_parent);
+				std::function<rom_entry const * ()> np(next_parent); // important - copy mutable lambda
 				for (rom_entry const *parent = np(); parent; parent = np())
 				{
 					parent = rom_first_region(parent);
@@ -235,7 +255,7 @@ auto open_parent_disk(
 							parent = rom_next_region(parent);
 						if (parent)
 						{
-							rom_entry const *romp = rom_first_file(parent);
+							rom_entry const *romp(rom_first_file(parent));
 							while (romp)
 							{
 								if (util::hash_collection(romp->hashdata()) == hashes)
@@ -333,7 +353,7 @@ const rom_entry *rom_first_parameter(const device_t &device)
 	const rom_entry *romp = &device.rom_region_vector().front();
 	while (romp && !ROMENTRY_ISEND(romp) && !ROMENTRY_ISPARAMETER(romp))
 		romp++;
-	return (romp != nullptr && !ROMENTRY_ISEND(romp)) ? romp : nullptr;
+	return (romp && !ROMENTRY_ISEND(romp)) ? romp : nullptr;
 }
 
 
@@ -358,17 +378,17 @@ const rom_entry *rom_next_parameter(const rom_entry *romp)
 
 u32 rom_file_size(const rom_entry *romp)
 {
-	u32 maxlength = 0;
+	u32 maxlength(0);
 
-	/* loop until we run out of reloads */
+	// loop until we run out of reloads
 	do
 	{
-		/* loop until we run out of continues/ignores */
+		// loop until we run out of continues/ignores
 		u32 curlength = ROM_GETLENGTH(romp++);
 		while (ROMENTRY_ISCONTINUE(romp) || ROMENTRY_ISIGNORE(romp))
 			curlength += ROM_GETLENGTH(romp++);
 
-		/* track the maximum length */
+		// track the maximum length
 		maxlength = std::max(maxlength, curlength);
 	}
 	while (ROMENTRY_ISRELOAD(romp));
@@ -377,27 +397,6 @@ u32 rom_file_size(const rom_entry *romp)
 }
 
 
-/*-------------------------------------------------
-    debugload - log data to a file
--------------------------------------------------*/
-
-static void CLIB_DECL ATTR_PRINTF(1,2) debugload(const char *string, ...)
-{
-	static int opened;
-	va_list arg;
-	FILE *f;
-
-	f = fopen("romload.log", opened++ ? "a" : "w");
-	if (f)
-	{
-		va_start(arg, string);
-		vfprintf(f, string, arg);
-		va_end(arg);
-		fclose(f);
-	}
-}
-
-
 /***************************************************************************
     HARD DISK HANDLING
 ***************************************************************************/
@@ -410,8 +409,10 @@ static void CLIB_DECL ATTR_PRINTF(1,2) debugload(const char *string, ...)
 chd_file *rom_load_manager::get_disk_handle(std::string_view region)
 {
 	for (auto &curdisk : m_chd_list)
+	{
 		if (curdisk->region() == region)
 			return &curdisk->chd();
+	}
 	return nullptr;
 }
 
@@ -421,12 +422,12 @@ chd_file *rom_load_manager::get_disk_handle(std::string_view region)
     file associated with the given region
 -------------------------------------------------*/
 
-std::error_condition rom_load_manager::set_disk_handle(std::string_view region, const char *fullpath)
+std::error_condition rom_load_manager::set_disk_handle(std::string_view region, std::string_view fullpath)
 {
-	auto chd = std::make_unique<open_chd>(region);
+	auto chd(std::make_unique<open_chd>(region));
-	auto err = chd->orig_chd().open(fullpath);
+	std::error_condition const err(chd->orig_chd().open(fullpath));
 	if (!err)
-		m_chd_list.push_back(std::move(chd));
+		m_chd_list.emplace_back(std::move(chd));
 	return err;
 }
 
@@ -442,7 +443,7 @@ void rom_load_manager::determine_bios_rom(device_t &device, std::string_view spe
 	if (!specbios.empty() && !util::streqlower(specbios, "default"sv))
 	{
 		bool found(false);
-		for (const rom_entry &rom : device.rom_region_vector())
+		for (rom_entry const &rom : device.rom_region_vector())
 		{
 			if (ROMENTRY_ISSYSTEM_BIOS(&rom))
 			{
@@ -450,7 +451,7 @@ void rom_load_manager::determine_bios_rom(device_t &device, std::string_view spe
 				int const bios_flags = ROM_GETBIOSFLAGS(&rom);
 
 				// Allow '-bios n' to still be used
-				if (specbios == std::to_string(bios_flags - 1) || util::streqlower(specbios, biosname))
+				if ((specbios == std::to_string(bios_flags - 1)) || util::streqlower(specbios, biosname))
 				{
 					found = true;
 					device.set_system_bios(bios_flags);
@@ -479,21 +480,29 @@ void rom_load_manager::determine_bios_rom(device_t &device, std::string_view spe
 
 void rom_load_manager::count_roms()
 {
-	const rom_entry *region, *rom;
+	// start with 0
-
-	/* start with 0 */
 	m_romstotal = 0;
 	m_romstotalsize = 0;
 
-	/* loop over regions, then over files */
+	// loop over regions, and files within regions
 	for (device_t &device : device_enumerator(machine().config().root_device()))
-		for (region = rom_first_region(device); region != nullptr; region = rom_next_region(region))
+	{
-			for (rom = rom_first_file(region); rom != nullptr; rom = rom_next_file(rom))
+		rom_entry const *region(rom_first_region(device));
-				if (ROM_GETBIOSFLAGS(rom) == 0 || ROM_GETBIOSFLAGS(rom) == device.system_bios())
+		while (region)
+		{
+			rom_entry const *rom(rom_first_file(region));
+			while (rom)
+			{
+				if ((ROM_GETBIOSFLAGS(rom) == 0) || (ROM_GETBIOSFLAGS(rom) == device.system_bios()))
 				{
 					m_romstotal++;
 					m_romstotalsize += rom_file_size(rom);
 				}
+				rom = rom_next_file(rom);
+			}
+			region = rom_next_region(region);
+		}
+	}
 }
 
 
@@ -531,7 +540,7 @@ void rom_load_manager::handle_missing_file(const rom_entry *romp, const std::vec
 	const bool is_chd(chderr);
 	const std::string name(is_chd ? romp->name() + ".chd" : romp->name());
 
-	const bool is_chd_error(is_chd && chderr != std::errc::no_such_file_or_directory);
+	const bool is_chd_error(is_chd && (chderr != std::errc::no_such_file_or_directory));
 	if (is_chd_error)
 		m_errorstring.append(string_format("%s CHD ERROR: %s\n", name, chderr.message()));
 
@@ -584,7 +593,7 @@ void rom_load_manager::verify_length_and_hash(emu_file *file, std::string_view n
 		return;
 
 	// verify length
-	u64 const actlength = file->size();
+	u64 const actlength(file->size());
 	if (explength != actlength)
 	{
 		m_errorstring.append(string_format("%s WRONG LENGTH (expected: %08x found: %08x)\n", name, explength, actlength));
@@ -600,7 +609,7 @@ void rom_load_manager::verify_length_and_hash(emu_file *file, std::string_view n
 	else
 	{
 		// verify checksums
-		util::hash_collection const &acthashes = file->hashes(hashes.hash_types());
+		util::hash_collection const &acthashes(file->hashes(hashes.hash_types()));
 		if (hashes != acthashes)
 		{
 			// otherwise, it's just bad
@@ -646,19 +655,19 @@ void rom_load_manager::display_loading_rom_message(const char *name, bool from_l
 
 void rom_load_manager::display_rom_load_results(bool from_list)
 {
-	/* final status display */
+	// final status display
 	display_loading_rom_message(nullptr, from_list);
 
-	/* if we had errors, they are fatal */
+	// if we had errors, they are fatal
 	if (m_errors != 0)
 	{
-		/* create the error message and exit fatally */
+		// create the error message and exit fatally
 		osd_printf_error("%s", m_errorstring);
 		throw emu_fatalerror(EMU_ERR_MISSING_FILES, "Required files are missing, the machine cannot be run.");
 	}
 
-	/* if we had warnings, output them, but continue */
+	// if we had warnings, output them, but continue
-	if ((m_warnings) || (m_knownbad))
+	if (m_warnings || m_knownbad)
 	{
 		m_errorstring.append("WARNING: the machine might not run correctly.");
 		osd_printf_warning("%s\n", m_errorstring);
@@ -674,27 +683,28 @@ void rom_load_manager::display_rom_load_results(bool from_list)
 void rom_load_manager::region_post_process(memory_region *region, bool invert)
 {
 	// do nothing if no region
-	if (region == nullptr)
+	if (!region)
 		return;
 
-	LOG("+ datawidth=%dbit endian=%s\n", region->bitwidth(),
+	LOG("+ datawidth=%dbit endian=%s\n",
-			region->endianness() == ENDIANNESS_LITTLE ? "little" : "big");
+			region->bitwidth(),
+			(region->endianness() == ENDIANNESS_LITTLE) ? "little" : "big");
 
-	/* if the region is inverted, do that now */
+	// if the region is inverted, do that now
 	if (invert)
 	{
 		LOG("+ Inverting region\n");
-		u8 *base = region->base();
+		u8 *base(region->base());
 		for (int i = 0; i < region->bytes(); i++)
 			*base++ ^= 0xff;
 	}
 
-	/* swap the endianness if we need to */
+	// swap the endianness if we need to
-	if (region->bytewidth() > 1 && region->endianness() != ENDIANNESS_NATIVE)
+	int const datawidth(region->bytewidth());
+	if ((datawidth > 1) && (region->endianness() != ENDIANNESS_NATIVE))
 	{
 		LOG("+ Byte swapping region\n");
-		int datawidth = region->bytewidth();
+		u8 *base(region->base());
-		u8 *base = region->base();
 		for (int i = 0; i < region->bytes(); i += datawidth)
 		{
 			u8 temp[8];
@@ -800,48 +810,46 @@ int rom_load_manager::read_rom_data(
 		const rom_entry *parent_region,
 		const rom_entry *romp)
 {
-	int datashift = ROM_GETBITSHIFT(romp);
+	int const datashift(ROM_GETBITSHIFT(romp));
-	int datamask = ((1 << ROM_GETBITWIDTH(romp)) - 1) << datashift;
+	int const datamask(((1 << ROM_GETBITWIDTH(romp)) - 1) << datashift);
-	int numbytes = ROM_GETLENGTH(romp);
+	int numbytes(ROM_GETLENGTH(romp));
-	int groupsize = ROM_GETGROUPSIZE(romp);
+	int const groupsize(ROM_GETGROUPSIZE(romp));
-	int skip = ROM_GETSKIPCOUNT(romp);
+	int skip(ROM_GETSKIPCOUNT(romp));
-	int reversed = ROM_ISREVERSED(romp);
+	int const reversed(ROM_ISREVERSED(romp));
-	int numgroups = (numbytes + groupsize - 1) / groupsize;
+	int const numgroups((numbytes + groupsize - 1) / groupsize);
 	u8 *base = region.base() + ROM_GETOFFSET(romp);
-	u32 tempbufsize;
-	int i;
 
 	LOG("Loading ROM data: offs=%X len=%X mask=%02X group=%d skip=%d reverse=%d\n", ROM_GETOFFSET(romp), numbytes, datamask, groupsize, skip, reversed);
 
-	/* make sure the length was an even multiple of the group size */
+	// make sure the length was a whole multiple of the group size
-	if (numbytes % groupsize != 0)
+	if ((numbytes % groupsize) != 0)
 		osd_printf_warning("Warning in RomModule definition: %s length not an even multiple of group size\n", romp->name());
 
-	/* make sure we only fill within the region space */
+	// make sure we only fill within the region space
 	if (ROM_GETOFFSET(romp) + numgroups * groupsize + (numgroups - 1) * skip > region.bytes())
 		throw emu_fatalerror("Error in RomModule definition: %s out of memory region space\n", romp->name());
 
-	/* make sure the length was valid */
+	// make sure the length was valid
 	if (numbytes == 0)
 		throw emu_fatalerror("Error in RomModule definition: %s has an invalid length\n", romp->name());
 
-	/* special case for simple loads */
+	// special case for simple loads
-	if (datamask == 0xff && (groupsize == 1 || !reversed) && skip == 0)
+	if ((datamask == 0xff) && ((groupsize == 1) || !reversed) && (skip == 0))
 		return rom_fread(file, base, numbytes, parent_region);
 
-	/* use a temporary buffer for complex loads */
+	// use a temporary buffer for complex loads
-	tempbufsize = std::min(TEMPBUFFER_MAX_SIZE, numbytes);
+	u32 const tempbufsize(std::min(TEMPBUFFER_MAX_SIZE, numbytes));
-	std::vector<u8> tempbuf(tempbufsize);
+	std::unique_ptr<u8 []> const tempbuf(new u8 [tempbufsize]);
 
-	/* chunky reads for complex loads */
+	// chunky reads for complex loads
 	skip += groupsize;
 	while (numbytes > 0)
 	{
-		int evengroupcount = (tempbufsize / groupsize) * groupsize;
+		int evengroupcount((tempbufsize / groupsize) * groupsize);
-		int bytesleft = (numbytes > evengroupcount) ? evengroupcount : numbytes;
+		int bytesleft((numbytes > evengroupcount) ? evengroupcount : numbytes);
-		u8 *bufptr = &tempbuf[0];
+		u8 *bufptr(tempbuf.get());
 
-		/* read as much as we can */
+		// read as much as we can
 		LOG("  Reading %X bytes into buffer\n", bytesleft);
 		if (rom_fread(file, bufptr, bytesleft, parent_region) != bytesleft)
 			return 0;
@@ -849,58 +857,65 @@ int rom_load_manager::read_rom_data(
 
 		LOG("  Copying to %p\n", base);
 
-		/* unmasked cases */
 		if (datamask == 0xff)
 		{
-			/* non-grouped data */
+			// unmasked cases
 			if (groupsize == 1)
-				for (i = 0; i < bytesleft; i++, base += skip)
+			{
+				// non-grouped data
+				for (int i = 0; i < bytesleft; i++, base += skip)
 					*base = *bufptr++;
-
+			}
-			/* grouped data -- non-reversed case */
 			else if (!reversed)
+			{
+				// grouped data -- non-reversed case
 				while (bytesleft)
 				{
-					for (i = 0; i < groupsize && bytesleft; i++, bytesleft--)
+					for (int i = 0; i < groupsize && bytesleft; i++, bytesleft--)
 						base[i] = *bufptr++;
 					base += skip;
 				}
-
+			}
-			/* grouped data -- reversed case */
 			else
+			{
+				// grouped data -- reversed case
 				while (bytesleft)
 				{
-					for (i = groupsize - 1; i >= 0 && bytesleft; i--, bytesleft--)
+					for (int i = groupsize - 1; i >= 0 && bytesleft; i--, bytesleft--)
 						base[i] = *bufptr++;
 					base += skip;
 				}
+			}
 		}
-
-		/* masked cases */
 		else
 		{
-			/* non-grouped data */
+			// masked cases
 			if (groupsize == 1)
-				for (i = 0; i < bytesleft; i++, base += skip)
+			{
+				// non-grouped data
+				for (int i = 0; i < bytesleft; i++, base += skip)
 					*base = (*base & ~datamask) | ((*bufptr++ << datashift) & datamask);
-
+			}
-			/* grouped data -- non-reversed case */
 			else if (!reversed)
+			{
+				// grouped data -- non-reversed case
 				while (bytesleft)
 				{
-					for (i = 0; i < groupsize && bytesleft; i++, bytesleft--)
+					for (int i = 0; i < groupsize && bytesleft; i++, bytesleft--)
 						base[i] = (base[i] & ~datamask) | ((*bufptr++ << datashift) & datamask);
 					base += skip;
 				}
-
+			}
-			/* grouped data -- reversed case */
 			else
+			{
+				// grouped data -- reversed case
 				while (bytesleft)
 				{
-					for (i = groupsize - 1; i >= 0 && bytesleft; i--, bytesleft--)
+					for (int i = groupsize - 1; i >= 0 && bytesleft; i--, bytesleft--)
 						base[i] = (base[i] & ~datamask) | ((*bufptr++ << datashift) & datamask);
 					base += skip;
 				}
+			}
 		}
 	}
 

src/emu/romload.h

@@ -17,7 +17,9 @@
 
 #include <functional>
 #include <initializer_list>
+#include <memory>
 #include <string>
+#include <string_view>
 #include <system_error>
 #include <type_traits>
 #include <vector>
@@ -421,7 +423,7 @@ public:
 	chd_file *get_disk_handle(std::string_view region);
 
 	/* set a pointer to the CHD file associated with the given region */
-	std::error_condition set_disk_handle(std::string_view region, const char *fullpath);
+	std::error_condition set_disk_handle(std::string_view region, std::string_view fullpath);
 
 	void load_software_part_region(device_t &device, software_list_device &swlist, std::string_view swname, const rom_entry *start_region);
 

src/mame/virtual/ldplayer.cpp

@@ -192,7 +192,7 @@ chd_file *ldplayer_state::get_disc()
 
 				// try to open the CHD
 
-				if (!machine().rom_load().set_disk_handle("laserdisc", fullpath.c_str()))
+				if (!machine().rom_load().set_disk_handle("laserdisc", fullpath))
 				{
 					m_filename.assign(dir->name);
 					found = true;