chd.cpp: Refactoring, part 2

- Change is_XXX to return std::error_condition instead of bool so that errors can be passed down the line; rename these to check_is_XXX to indicate that they are no longer basic predicates
- Change return type of internal function metadata_find to std::error_condition so that errors can be returned rather than thrown

* imagedev/cdromimg.cpp: Fix bug where cdrom_file object could be constructed twice in a row for CD-ROMs and once for DVDs

src/devices/imagedev/cdromimg.cpp

@@ -79,9 +79,10 @@ void cdrom_image_device::setup_current_preset_image()
 	m_dvdrom_handle.reset();
 
 	chd_file *chd = current_preset_image_chd();
-	if (chd->is_cd() || (m_gd_compat && chd->is_gd()))
+
+	if (!chd->check_is_cd() || (m_gd_compat && !chd->check_is_gd()))
 		m_cdrom_handle = std::make_unique<cdrom_file>(chd);
-	else if(m_dvd_compat && chd->is_dvd())
+	else if(m_dvd_compat && !chd->check_is_dvd())
 		m_dvdrom_handle = std::make_unique<dvdrom_file>(chd);
 	else
 		fatalerror("chd for region %s is not compatible with the cdrom image device\n", preset_images_list()[current_preset_image_id()]);
@@ -126,16 +127,31 @@ std::pair<std::error_condition, std::string> cdrom_image_device::call_load()
 	// open the CHD file
 	if (chd)
 	{
-		if (chd->is_cd() || (m_gd_compat && chd->is_gd()))
+		err = chd->check_is_cd();
-			m_cdrom_handle.reset(new cdrom_file(chd));
+		if (err == chd_file::error::METADATA_NOT_FOUND && m_gd_compat)
-		else if(m_dvd_compat && chd->is_dvd())
+			err = chd->check_is_gd();
-			m_dvdrom_handle.reset(new dvdrom_file(chd));
+		if (!err)
-		else
 		{
-			err = image_error::UNSUPPORTED;
+			m_cdrom_handle.reset(new cdrom_file(chd));
+			return std::make_pair(std::error_condition(), std::string());
+		}
+		if (err != chd_file::error::METADATA_NOT_FOUND)
+			goto error;
+
+		if (m_dvd_compat)
+		{
+			err = chd->check_is_dvd();
+			if (!err)
+			{
+				m_dvdrom_handle.reset(new dvdrom_file(chd));
+				return std::make_pair(std::error_condition(), std::string());
+			}
+			if (err != chd_file::error::METADATA_NOT_FOUND)
 				goto error;
 		}
-		m_cdrom_handle.reset(new cdrom_file(chd));
+
+		err = image_error::UNSUPPORTED;
+		goto error;
 	}
 	else
 	{

src/lib/util/chd.cpp

@@ -1292,19 +1292,19 @@ std::error_condition chd_file::write_bytes(uint64_t offset, const void *buffer,
  * @param   searchindex     The searchindex.
  * @param [in,out]  output  The output.
  *
- * @return  The metadata.
+ * @return  A std::error_condition.
  */
 
 std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::string &output)
 {
+	// if we didn't find it, just return
+	metadata_entry metaentry;
+	if (std::error_condition err = metadata_find(searchtag, searchindex, metaentry))
+		return err;
+
 	// wrap this for clean reporting
 	try
 	{
-		// if we didn't find it, just return
-		metadata_entry metaentry;
-		if (!metadata_find(searchtag, searchindex, metaentry))
-			return std::error_condition(error::METADATA_NOT_FOUND);
-
 		// read the metadata
 		output.assign(metaentry.length, '\0');
 		file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
@@ -1329,19 +1329,19 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
  * @param   searchindex     The searchindex.
  * @param [in,out]  output  The output.
  *
- * @return  The metadata.
+ * @return  A std::error_condition.
  */
 
 std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::vector<uint8_t> &output)
 {
+	// if we didn't find it, just return
+	metadata_entry metaentry;
+	if (std::error_condition err = metadata_find(searchtag, searchindex, metaentry))
+		return err;
+
 	// wrap this for clean reporting
 	try
 	{
-		// if we didn't find it, just return
-		metadata_entry metaentry;
-		if (!metadata_find(searchtag, searchindex, metaentry))
-			throw std::error_condition(error::METADATA_NOT_FOUND);
-
 		// read the metadata
 		output.resize(metaentry.length);
 		file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
@@ -1368,19 +1368,19 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
  * @param   outputlen           The outputlen.
  * @param [in,out]  resultlen   The resultlen.
  *
- * @return  The metadata.
+ * @return  A std::error_condition.
  */
 
 std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, void *output, uint32_t outputlen, uint32_t &resultlen)
 {
+	// if we didn't find it, just return
+	metadata_entry metaentry;
+	if (std::error_condition err = metadata_find(searchtag, searchindex, metaentry))
+		return err;
+
 	// wrap this for clean reporting
 	try
 	{
-		// if we didn't find it, just return
-		metadata_entry metaentry;
-		if (!metadata_find(searchtag, searchindex, metaentry))
-			throw std::error_condition(error::METADATA_NOT_FOUND);
-
 		// read the metadata
 		resultlen = metaentry.length;
 		file_read(metaentry.offset + METADATA_HEADER_SIZE, output, std::min(outputlen, resultlen));
@@ -1407,19 +1407,19 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
  * @param [in,out]  resulttag   The resulttag.
  * @param [in,out]  resultflags The resultflags.
  *
- * @return  The metadata.
+ * @return  A std::error_condition.
  */
 
 std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::vector<uint8_t> &output, chd_metadata_tag &resulttag, uint8_t &resultflags)
 {
+	// if we didn't find it, just return
+	metadata_entry metaentry;
+	if (std::error_condition err = metadata_find(searchtag, searchindex, metaentry))
+		return err;
+
 	// wrap this for clean reporting
 	try
 	{
-		// if we didn't find it, just return
-		metadata_entry metaentry;
-		if (!metadata_find(searchtag, searchindex, metaentry))
-			throw std::error_condition(error::METADATA_NOT_FOUND);
-
 		// read the metadata
 		output.resize(metaentry.length);
 		file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
@@ -1462,7 +1462,8 @@ std::error_condition chd_file::write_metadata(chd_metadata_tag metatag, uint32_t
 		// find the entry if it already exists
 		metadata_entry metaentry;
 		bool finished = false;
-		if (metadata_find(metatag, metaindex, metaentry))
+		std::error_condition err = metadata_find(metatag, metaindex, metaentry);
+		if (!err)
 		{
 			// if the new data fits over the old data, just overwrite
 			if (inputlen <= metaentry.length)
@@ -1485,6 +1486,8 @@ std::error_condition chd_file::write_metadata(chd_metadata_tag metatag, uint32_t
 			else
 				metadata_set_previous_next(metaentry.prev, metaentry.next);
 		}
+		else if (err != error::METADATA_NOT_FOUND)
+			throw err;
 
 		// if not yet done, create a new entry and append
 		if (!finished)
@@ -1533,14 +1536,14 @@ std::error_condition chd_file::write_metadata(chd_metadata_tag metatag, uint32_t
 
 std::error_condition chd_file::delete_metadata(chd_metadata_tag metatag, uint32_t metaindex)
 {
+	// find the entry
+	metadata_entry metaentry;
+	if (std::error_condition err = metadata_find(metatag, metaindex, metaentry))
+		return err;
+
 	// wrap this for clean reporting
 	try
 	{
-		// find the entry
-		metadata_entry metaentry;
-		if (!metadata_find(metatag, metaindex, metaentry))
-			throw std::error_condition(error::METADATA_NOT_FOUND);
-
 		// point the previous to the next, unlinking us
 		metadata_set_previous_next(metaentry.prev, metaentry.next);
 		return std::error_condition();
@@ -1568,9 +1571,6 @@ std::error_condition chd_file::delete_metadata(chd_metadata_tag metatag, uint32_
 
 std::error_condition chd_file::clone_all_metadata(chd_file &source)
 {
-	// wrap this for clean reporting
-	try
-	{
 	// iterate over metadata entries in the source
 	std::vector<uint8_t> filedata;
 	metadata_entry metaentry;
@@ -1578,24 +1578,30 @@ std::error_condition chd_file::clone_all_metadata(chd_file &source)
 	metaentry.length = 0;
 	metaentry.next = 0;
 	metaentry.flags = 0;
-		for (bool has_data = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry); has_data; has_data = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry, true))
+	std::error_condition err;
+	for (err = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry); !err; err = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry, true))
+	{
+		// wrap this for clean reporting
+		try
 		{
 			// read the metadata item
 			filedata.resize(metaentry.length);
 			source.file_read(metaentry.offset + METADATA_HEADER_SIZE, &filedata[0], metaentry.length);
+		}
+		catch (std::error_condition const &filerr)
+		{
+			return filerr;
+		}
 	
 		// write it to the destination
-			std::error_condition err = write_metadata(metaentry.metatag, (uint32_t)-1, &filedata[0], metaentry.length, metaentry.flags);
+		err = write_metadata(metaentry.metatag, (uint32_t)-1, &filedata[0], metaentry.length, metaentry.flags);
 		if (err)
-				throw err;
-		}
-		return std::error_condition();
-	}
-	catch (std::error_condition const &err)
-	{
-		// return any errors
 			return err;
 	}
+	if (err == error::METADATA_NOT_FOUND)
+		return std::error_condition();
+	else
+		return err;
 }
 
 /**
@@ -1621,7 +1627,8 @@ util::sha1_t chd_file::compute_overall_sha1(util::sha1_t rawsha1)
 	std::vector<uint8_t> filedata;
 	std::vector<metadata_hash> hasharray;
 	metadata_entry metaentry;
-	for (bool has_data = metadata_find(CHDMETATAG_WILDCARD, 0, metaentry); has_data; has_data = metadata_find(CHDMETATAG_WILDCARD, 0, metaentry, true))
+	std::error_condition err;
+	for (err = metadata_find(CHDMETATAG_WILDCARD, 0, metaentry); !err; err = metadata_find(CHDMETATAG_WILDCARD, 0, metaentry, true))
 	{
 		// if not checksumming, continue
 		if ((metaentry.flags & CHD_MDFLAGS_CHECKSUM) == 0)
@@ -1637,6 +1644,8 @@ util::sha1_t chd_file::compute_overall_sha1(util::sha1_t rawsha1)
 		hashentry.sha1 = util::sha1_creator::simple(&filedata[0], metaentry.length);
 		hasharray.push_back(hashentry);
 	}
+	if (err != error::METADATA_NOT_FOUND)
+		throw err;
 
 	// sort the array
 	if (!hasharray.empty())
@@ -2658,7 +2667,7 @@ void chd_file::hunk_copy_from_parent(uint32_t hunknum, uint64_t parentunit)
 }
 
 /**
- * @fn  bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume)
+ * @fn  std::error_condition chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume)
  *
  * @brief   -------------------------------------------------
  *            metadata_find - find a metadata entry
@@ -2669,10 +2678,10 @@ void chd_file::hunk_copy_from_parent(uint32_t hunknum, uint64_t parentunit)
  * @param [in,out]  metaentry   The metaentry.
  * @param   resume              true to resume.
  *
- * @return  true if it succeeds, false if it fails.
+ * @return  A std::error_condition (error::METADATA_NOT_FOUND if the search fails).
  */
 
-bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume) const
+std::error_condition chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume) const
 {
 	// start at the beginning unless we're resuming a previous search
 	if (!resume)
@@ -2686,6 +2695,9 @@ bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metada
 		metaentry.offset = metaentry.next;
 	}
 
+	// wrap this for clean reporting
+	try
+	{
 		// loop until we run out of options
 		while (metaentry.offset != 0)
 		{
@@ -2702,7 +2714,7 @@ bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metada
 			// if we got a match, proceed
 			if (metatag == CHDMETATAG_WILDCARD || metaentry.metatag == metatag)
 				if (metaindex-- == 0)
-				return true;
+					return std::error_condition();
 
 			// no match, fetch the next link
 			metaentry.prev = metaentry.offset;
@@ -2710,7 +2722,13 @@ bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metada
 		}
 
 		// if we get here, we didn't find it
-	return false;
+		return error::METADATA_NOT_FOUND;
+	}
+	catch (std::error_condition const &err)
+	{
+		// return any errors
+		return err;
+	}
 }
 
 /**
@@ -3344,71 +3362,40 @@ void chd_file_compressor::hashmap::add(uint64_t itemnum, util::crc16_t crc16, ut
 	m_map[crc16] = entry;
 }
 
-bool chd_file::is_hd() const
+std::error_condition chd_file::check_is_hd() const
 {
-	try
-	{
 	metadata_entry metaentry;
 	return metadata_find(HARD_DISK_METADATA_TAG, 0, metaentry);
-	}
-	catch (std::error_condition const &)
-	{
-		return false;
-	}
 }
 
-bool chd_file::is_cd() const
+std::error_condition chd_file::check_is_cd() const
 {
-	try
-	{
 	metadata_entry metaentry;
-		return metadata_find(CDROM_OLD_METADATA_TAG, 0, metaentry)
+	std::error_condition err = metadata_find(CDROM_OLD_METADATA_TAG, 0, metaentry);
-			|| metadata_find(CDROM_TRACK_METADATA_TAG, 0, metaentry)
+	if (err == error::METADATA_NOT_FOUND)
-			|| metadata_find(CDROM_TRACK_METADATA2_TAG, 0, metaentry);
+		err = metadata_find(CDROM_TRACK_METADATA_TAG, 0, metaentry);
-	}
+	if (err == error::METADATA_NOT_FOUND)
-	catch (std::error_condition const &)
+		err = metadata_find(CDROM_TRACK_METADATA2_TAG, 0, metaentry);
-	{
+	return err;
-		return false;
-	}
 }
 
-bool chd_file::is_gd() const
+std::error_condition chd_file::check_is_gd() const
 {
-	try
-	{
 	metadata_entry metaentry;
-		return metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry)
+	std::error_condition err = metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry);
-			|| metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry);
+	if (err == error::METADATA_NOT_FOUND)
-	}
+		err = metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry);
-	catch (std::error_condition const &)
+	return err;
-	{
-		return false;
-	}
 }
 
-bool chd_file::is_dvd() const
+std::error_condition chd_file::check_is_dvd() const
 {
-	try
-	{
 	metadata_entry metaentry;
 	return metadata_find(DVD_METADATA_TAG, 0, metaentry);
-	}
-	catch (std::error_condition const &)
-	{
-		return false;
-	}
 }
 
-bool chd_file::is_av() const
+std::error_condition chd_file::check_is_av() const
 {
-	try
-	{
 	metadata_entry metaentry;
 	return metadata_find(AV_METADATA_TAG, 0, metaentry);
-	}
-	catch (std::error_condition const &)
-	{
-		return false;
-	}
 }
-

src/lib/util/chd.h

@@ -361,11 +361,11 @@ public:
 	std::error_condition codec_configure(chd_codec_type codec, int param, void *config);
 
 	// typing
-	bool is_hd() const;
+	std::error_condition check_is_hd() const;
-	bool is_cd() const;
+	std::error_condition check_is_cd() const;
-	bool is_gd() const;
+	std::error_condition check_is_gd() const;
-	bool is_dvd() const;
+	std::error_condition check_is_dvd() const;
-	bool is_av() const;
+	std::error_condition check_is_av() const;
 
 private:
 	struct metadata_entry;
@@ -393,7 +393,7 @@ private:
 	void hunk_write_compressed(uint32_t hunknum, int8_t compression, const uint8_t *compressed, uint32_t complength, util::crc16_t crc16);
 	void hunk_copy_from_self(uint32_t hunknum, uint32_t otherhunk);
 	void hunk_copy_from_parent(uint32_t hunknum, uint64_t parentunit);
-	bool metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume = false) const;
+	std::error_condition metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume = false) const;
 	void metadata_set_previous_next(uint64_t prevoffset, uint64_t nextoffset);
 	void metadata_update_hash();
 	static int CLIB_DECL metadata_hash_compare(const void *elem1, const void *elem2);

src/lib/util/dvdrom.cpp

@@ -58,7 +58,7 @@ dvdrom_file::dvdrom_file(chd_file *_chd)
 		throw nullptr;
 
 	/* check it's actually a DVD-ROM */
-	if (!chd->is_dvd())
+	if (chd->check_is_dvd())
 		throw nullptr;
 
 	sector_count = chd->unit_count();