Merge pull request #1158 from npwoods/misc_cleanups

Miscellaneous cleanups

src/emu/diimage.cpp

@@ -184,36 +184,38 @@ void device_image_interface::device_compute_hash(util::hash_collection &hashes,
 //  an image
 //-------------------------------------------------
 
-image_error_t device_image_interface::set_image_filename(const std::string &filename)
+void device_image_interface::set_image_filename(const std::string &filename)
 {
 	m_image_name = filename;
-	util::zippath_parent(m_working_directory, filename.c_str());
+	util::zippath_parent(m_working_directory, filename);
 	m_basename.assign(m_image_name);
 
-	int loc1 = m_image_name.find_last_of('\\');
+	// find the last "path separator"
-	int loc2 = m_image_name.find_last_of('/');
+	auto iter = std::find_if(
-	int loc3 = m_image_name.find_last_of(':');
+		m_image_name.rbegin(),
-	int loc = MAX(loc1, MAX(loc2, loc3));
+		m_image_name.rend(),
-	if (loc != -1) {
+		[](char c) { return (c == '\\') || (c == '/') || (c == ':'); });
-		if (loc == loc3)
+
+	if (iter != m_image_name.rend())
+	{
+		if (*iter == ':')
 		{
 			// temp workaround for softlists now that m_image_name contains the part name too (e.g. list:gamename:cart)
-			m_basename = m_basename.substr(0, loc);
+			m_basename.assign(m_image_name.begin(), std::next(iter).base());
 			int tmploc = m_basename.find_last_of(':');
-			m_basename = m_basename.substr(tmploc + 1, loc - tmploc);
+			m_basename = m_basename.substr(tmploc + 1);
 		}
 		else
-			m_basename = m_basename.substr(loc + 1);
+			m_basename.assign(iter.base(), m_image_name.end());
 	}
 	m_basename_noext = m_basename;
 	m_filetype = "";
-	loc = m_basename_noext.find_last_of('.');
+	auto loc = m_basename_noext.find_last_of('.');
-	if (loc != -1) {
+	if (loc != std::string::npos)
+	{
 		m_basename_noext = m_basename_noext.substr(0, loc);
 		m_filetype = m_basename.substr(loc + 1);
 	}
-
-	return IMAGE_ERROR_SUCCESS;
 }
 
 /****************************************************************************
@@ -641,6 +643,41 @@ bool device_image_interface::is_loaded()
 }
 
 
+//-------------------------------------------------
+//  image_error_from_file_error - converts an image
+//	error to a file error
+//-------------------------------------------------
+
+image_error_t device_image_interface::image_error_from_file_error(osd_file::error filerr)
+{
+	switch (filerr)
+	{
+	case osd_file::error::NONE:
+		return IMAGE_ERROR_SUCCESS;
+
+	case osd_file::error::NOT_FOUND:
+	case osd_file::error::ACCESS_DENIED:
+		// file not found (or otherwise cannot open)
+		return IMAGE_ERROR_FILENOTFOUND;
+
+	case osd_file::error::OUT_OF_MEMORY:
+		// out of memory
+		return IMAGE_ERROR_OUTOFMEMORY;
+
+	case osd_file::error::ALREADY_OPEN:
+		// this shouldn't happen
+		return IMAGE_ERROR_ALREADYOPEN;
+
+	case osd_file::error::FAILURE:
+	case osd_file::error::TOO_MANY_FILES:
+	case osd_file::error::INVALID_DATA:
+	default:
+		// other errors
+		return IMAGE_ERROR_INTERNAL;
+	}
+}
+
+
 //-------------------------------------------------
 //  load_image_by_path - loads an image with a
 //  specific path
@@ -648,104 +685,41 @@ bool device_image_interface::is_loaded()
 
 image_error_t device_image_interface::load_image_by_path(UINT32 open_flags, const std::string &path)
 {
-	image_error_t err;
 	std::string revised_path;
 
 	// attempt to read the file
 	auto const filerr = util::zippath_fopen(path, open_flags, m_file, revised_path);
+	if (filerr != osd_file::error::NONE)
+		return image_error_from_file_error(filerr);
 
-	// did the open succeed?
-	switch(filerr)
-	{
-		case osd_file::error::NONE:
-			// success!
 	m_readonly = (open_flags & OPEN_FLAG_WRITE) ? 0 : 1;
 	m_created = (open_flags & OPEN_FLAG_CREATE) ? 1 : 0;
-			err = IMAGE_ERROR_SUCCESS;
-			break;
-
-		case osd_file::error::NOT_FOUND:
-		case osd_file::error::ACCESS_DENIED:
-			// file not found (or otherwise cannot open); continue
-			err = IMAGE_ERROR_FILENOTFOUND;
-			break;
-
-		case osd_file::error::OUT_OF_MEMORY:
-			// out of memory
-			err = IMAGE_ERROR_OUTOFMEMORY;
-			break;
-
-		case osd_file::error::ALREADY_OPEN:
-			// this shouldn't happen
-			err = IMAGE_ERROR_ALREADYOPEN;
-			break;
-
-		case osd_file::error::FAILURE:
-		case osd_file::error::TOO_MANY_FILES:
-		case osd_file::error::INVALID_DATA:
-		default:
-			// other errors
-			err = IMAGE_ERROR_INTERNAL;
-			break;
-	}
-
-	// if successful, set the file name
-	if (filerr == osd_file::error::NONE)
 	set_image_filename(revised_path);
-
+	return IMAGE_ERROR_SUCCESS;
-	return err;
 }
 
+
+//-------------------------------------------------
+//  reopen_for_write
+//-------------------------------------------------
+
 int device_image_interface::reopen_for_write(const char *path)
 {
 	m_file.reset();
 
-	image_error_t err;
 	std::string revised_path;
 
 	// attempt to open the file for writing
 	auto const filerr = util::zippath_fopen(path, OPEN_FLAG_READ|OPEN_FLAG_WRITE|OPEN_FLAG_CREATE, m_file, revised_path);
+	if (filerr != osd_file::error::NONE)
+		return image_error_from_file_error(filerr);
 
-	// did the open succeed?
-	switch(filerr)
-	{
-		case osd_file::error::NONE:
 	// success!
 	m_readonly = 0;
 	m_created = 1;
-			err = IMAGE_ERROR_SUCCESS;
+	set_image_filename(revised_path);
-			break;
 
-		case osd_file::error::NOT_FOUND:
+	return IMAGE_ERROR_SUCCESS;
-		case osd_file::error::ACCESS_DENIED:
-			// file not found (or otherwise cannot open); continue
-			err = IMAGE_ERROR_FILENOTFOUND;
-			break;
-
-		case osd_file::error::OUT_OF_MEMORY:
-			// out of memory
-			err = IMAGE_ERROR_OUTOFMEMORY;
-			break;
-
-		case osd_file::error::ALREADY_OPEN:
-			// this shouldn't happen
-			err = IMAGE_ERROR_ALREADYOPEN;
-			break;
-
-		case osd_file::error::FAILURE:
-		case osd_file::error::TOO_MANY_FILES:
-		case osd_file::error::INVALID_DATA:
-		default:
-			// other errors
-			err = IMAGE_ERROR_INTERNAL;
-			break;
-	}
-
-	// if successful, set the file name
-	if (filerr == osd_file::error::NONE)
-		set_image_filename(revised_path.c_str());
-
-	return err;
 }
 
 
@@ -953,10 +927,7 @@ bool device_image_interface::load_internal(const std::string &path, bool is_crea
 	m_is_loading = true;
 
 	// record the filename
-	m_err = set_image_filename(path);
+	set_image_filename(path);
-
-	if (m_err)
-		goto done;
 
 	if (core_opens_image_file())
 	{
@@ -973,7 +944,7 @@ bool device_image_interface::load_internal(const std::string &path, bool is_crea
 				// if we had launched from softlist with a specified part, e.g. "shortname:part"
 				// we would have recorded the wrong name, so record it again based on software_info
 				if (m_software_info_ptr && !m_full_software_name.empty())
-					m_err = set_image_filename(m_full_software_name);
+					set_image_filename(m_full_software_name);
 
 				// check if image should be read-only
 				const char *read_only = get_feature("read_only");

src/emu/diimage.h

@@ -257,7 +257,7 @@ protected:
 	void clear();
 	bool is_loaded();
 
-	image_error_t set_image_filename(const std::string &filename);
+	void set_image_filename(const std::string &filename);
 
 	void clear_error();
 
@@ -280,11 +280,11 @@ protected:
 	static const image_device_type_info *find_device_type(iodevice_t type);
 	static const image_device_type_info m_device_info_array[];
 
-	/* error related info */
+	// error related info
 	image_error_t m_err;
 	std::string m_err_message;
 
-	/* variables that are only non-zero when an image is mounted */
+	// variables that are only non-zero when an image is mounted
 	util::core_file::ptr m_file;
 	std::unique_ptr<emu_file> m_mame_file;
 	std::string m_image_name;
@@ -292,27 +292,27 @@ protected:
 	std::string m_basename_noext;
 	std::string m_filetype;
 
-	/* working directory; persists across mounts */
+	// working directory; persists across mounts
 	std::string m_working_directory;
 
-	/* Software information */
+	// Software information
 	std::string m_full_software_name;
 	const software_info *m_software_info_ptr;
 	const software_part *m_software_part_ptr;
 	std::string m_software_list_name;
 
-	/* info read from the hash file/software list */
+	// info read from the hash file/software list
 	std::string m_longname;
 	std::string m_manufacturer;
 	std::string m_year;
 	UINT32  m_supported;
 
-	/* flags */
+	// flags
 	bool m_readonly;
 	bool m_created;
 	bool m_init_phase;
 
-	/* special - used when creating */
+	// special - used when creating
 	int m_create_format;
 	util::option_resolution *m_create_args;
 
@@ -321,14 +321,17 @@ protected:
 	std::string m_brief_instance_name;
 	std::string m_instance_name;
 
-	/* creation info */
+	// creation info
 	formatlist_type m_formatlist;
 
-	/* in the case of arcade cabinet with fixed carts inserted,
+	// in the case of arcade cabinet with fixed carts inserted,
-	 we want to disable command line cart loading... */
+	// we want to disable command line cart loading...
 	bool m_user_loadable;
 
 	bool m_is_loading;
+
+private:
+	static image_error_t image_error_from_file_error(osd_file::error filerr);
 };
 
 // iterator

src/frontend/mame/ui/imgcntrl.cpp

@@ -61,7 +61,7 @@ menu_control_device_image::menu_control_device_image(mame_ui_manager &mui, rende
 		if (m_image.exists())
 		{
 			m_current_file.assign(m_image.filename());
-			util::zippath_parent(m_current_directory, m_current_file.c_str());
+			util::zippath_parent(m_current_directory, m_current_file);
 		}
 		else
 		{