From beb81e484240c450932cd95e70fdfb6559f01eb2 Mon Sep 17 00:00:00 2001 From: AJR Date: Fri, 11 Oct 2024 20:58:12 -0400 Subject: [PATCH] chd.cpp: More API changes - Have metadata_find return std::error_condition instead of throwing an exception - Replace the is_XXX predicates with check_is_XXX methods that return a std::error_condition, enabling improved error reporting for cdrom_image_device - Retain read error information in chd_file_compressor - Make a bunch of methods noexcept This mostly restores the changes from cc772072fa635146b1df39a5694d2a8f8aa5a34f. --- src/devices/imagedev/cdromimg.cpp | 34 +++-- src/lib/util/chd.cpp | 200 +++++++++++++++++------------- src/lib/util/chd.h | 16 +-- src/lib/util/dvdrom.cpp | 4 +- src/tools/chdman.cpp | 6 +- 5 files changed, 149 insertions(+), 111 deletions(-) diff --git a/src/devices/imagedev/cdromimg.cpp b/src/devices/imagedev/cdromimg.cpp index 5848eb349dc..b4deb9fc4ad 100644 --- a/src/devices/imagedev/cdromimg.cpp +++ b/src/devices/imagedev/cdromimg.cpp @@ -79,9 +79,9 @@ 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(chd); - else if(m_dvd_compat && chd->is_dvd()) + else if(m_dvd_compat && !chd->check_is_dvd()) m_dvdrom_handle = std::make_unique(chd); else fatalerror("chd for region %s is not compatible with the cdrom image device\n", preset_images_list()[current_preset_image_id()]); @@ -130,15 +130,31 @@ std::pair cdrom_image_device::call_load() // open the CHD file if (chd) { - if (chd->is_cd() || (m_gd_compat && chd->is_gd())) - m_cdrom_handle.reset(new cdrom_file(chd)); - else if (m_dvd_compat && chd->is_dvd()) - m_dvdrom_handle.reset(new dvdrom_file(chd)); - else + err = chd->check_is_cd(); + if (err == chd_file::error::METADATA_NOT_FOUND && m_gd_compat) + err = chd->check_is_gd(); + if (!err) { - err = image_error::INVALIDIMAGE; - goto error; + 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; + } + + err = image_error::INVALIDIMAGE; + goto error; } else { diff --git a/src/lib/util/chd.cpp b/src/lib/util/chd.cpp index 90a8aece75d..1f663fedd2a 100644 --- a/src/lib/util/chd.cpp +++ b/src/lib/util/chd.cpp @@ -1290,14 +1290,14 @@ std::error_condition chd_file::write_bytes(uint64_t offset, const void *buffer, 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); @@ -1327,14 +1327,14 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::vector &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); @@ -1366,14 +1366,14 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_ 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)); @@ -1405,14 +1405,14 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::vector &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); @@ -1455,7 +1455,10 @@ 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 && err != error::METADATA_NOT_FOUND) + throw err; + else if (!err) { // if the new data fits over the old data, just overwrite if (inputlen <= metaentry.length) @@ -1526,14 +1529,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(); @@ -1561,34 +1564,37 @@ 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 filedata; + metadata_entry metaentry; + metaentry.metatag = 0; + metaentry.length = 0; + metaentry.next = 0; + metaentry.flags = 0; + std::error_condition err; + for (err = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry); !err; err = source.metadata_find(CHDMETATAG_WILDCARD, 0, metaentry, true)) { - // iterate over metadata entries in the source - std::vector filedata; - metadata_entry metaentry; - metaentry.metatag = 0; - 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)) + // 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); - - // write it to the destination - std::error_condition err = write_metadata(metaentry.metatag, (uint32_t)-1, &filedata[0], metaentry.length, metaentry.flags); - if (err) - throw err; } + catch (std::error_condition const &filerr) + { + return filerr; + } + + // write it to the destination + err = write_metadata(metaentry.metatag, (uint32_t)-1, &filedata[0], metaentry.length, metaentry.flags); + if (err) + return err; + } + if (err == error::METADATA_NOT_FOUND) return std::error_condition(); - } - catch (std::error_condition const &err) - { - // return any errors + else return err; - } } /** @@ -1614,7 +1620,8 @@ util::sha1_t chd_file::compute_overall_sha1(util::sha1_t rawsha1) std::vector filedata; std::vector 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) @@ -1630,6 +1637,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()) @@ -2661,7 +2670,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 @@ -2672,10 +2681,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 noexcept { // start at the beginning unless we're resuming a previous search if (!resume) @@ -2689,31 +2698,40 @@ bool chd_file::metadata_find(chd_metadata_tag metatag, int32_t metaindex, metada metaentry.offset = metaentry.next; } - // loop until we run out of options - while (metaentry.offset != 0) + // wrap this for clean reporting + try { - // read the raw header - uint8_t raw_meta_header[METADATA_HEADER_SIZE]; - file_read(metaentry.offset, raw_meta_header, sizeof(raw_meta_header)); + // loop until we run out of options + while (metaentry.offset != 0) + { + // read the raw header + uint8_t raw_meta_header[METADATA_HEADER_SIZE]; + file_read(metaentry.offset, raw_meta_header, sizeof(raw_meta_header)); - // extract the data - metaentry.metatag = get_u32be(&raw_meta_header[0]); - metaentry.flags = raw_meta_header[4]; - metaentry.length = get_u24be(&raw_meta_header[5]); - metaentry.next = get_u64be(&raw_meta_header[8]); + // extract the data + metaentry.metatag = get_u32be(&raw_meta_header[0]); + metaentry.flags = raw_meta_header[4]; + metaentry.length = get_u24be(&raw_meta_header[5]); + metaentry.next = get_u64be(&raw_meta_header[8]); - // if we got a match, proceed - if (metatag == CHDMETATAG_WILDCARD || metaentry.metatag == metatag) - if (metaindex-- == 0) - return true; + // if we got a match, proceed + if (metatag == CHDMETATAG_WILDCARD || metaentry.metatag == metatag) + if (metaindex-- == 0) + return std::error_condition(); - // no match, fetch the next link - metaentry.prev = metaentry.offset; - metaentry.offset = metaentry.next; + // no match, fetch the next link + metaentry.prev = metaentry.offset; + metaentry.offset = metaentry.next; + } + + // if we get here, we didn't find it + return error::METADATA_NOT_FOUND; + } + catch (std::error_condition const &err) + { + // return any errors + return err; } - - // if we get here, we didn't find it - return false; } /** @@ -2814,7 +2832,6 @@ chd_file_compressor::chd_file_compressor() m_read_queue(nullptr), m_read_queue_offset(0), m_read_done_offset(0), - m_read_error(false), m_work_queue(nullptr), m_write_hunk(0) { @@ -2868,7 +2885,7 @@ void chd_file_compressor::compress_begin() // reset read state m_read_queue_offset = 0; m_read_done_offset = 0; - m_read_error = false; + m_read_error.clear(); // reset work item state m_work_buffer.resize(hunk_bytes() * (WORK_BUFFER_HUNKS + 1)); @@ -2909,9 +2926,9 @@ void chd_file_compressor::compress_begin() std::error_condition chd_file_compressor::compress_continue(double &progress, double &ratio) { - // if we got an error, return an error + // if we got an error, return the error if (m_read_error) - return std::errc::io_error; + return m_read_error; // if done reading, queue some more while (m_read_queue_offset < m_logicalbytes && osd_work_queue_items(m_read_queue) < 2) @@ -2971,8 +2988,8 @@ std::error_condition chd_file_compressor::compress_continue(double &progress, do // writes of all-0 data don't actually take space, so see if we count this chd_codec_type codec = CHD_CODEC_NONE; uint32_t complen; - hunk_info(item.m_hunknum, codec, complen); - if (codec == CHD_CODEC_NONE) + err = hunk_info(item.m_hunknum, codec, complen); + if (!err && codec == CHD_CODEC_NONE) m_total_out += m_hunkbytes; } else do @@ -3232,12 +3249,12 @@ void chd_file_compressor::async_read() catch (std::error_condition const &err) { fprintf(stderr, "CHD error occurred: %s\n", err.message().c_str()); - m_read_error = true; + m_read_error = err; } catch (std::exception const &ex) { fprintf(stderr, "exception occurred: %s\n", ex.what()); - m_read_error = true; + m_read_error = std::errc::io_error; // TODO: revisit this error code } } @@ -3312,7 +3329,7 @@ void chd_file_compressor::hashmap::reset() * @return An uint64_t. */ -uint64_t chd_file_compressor::hashmap::find(util::crc16_t crc16, util::sha1_t sha1) +uint64_t chd_file_compressor::hashmap::find(util::crc16_t crc16, util::sha1_t sha1) const noexcept { // look up the entry in the map for (entry_t *entry = m_map[crc16]; entry != nullptr; entry = entry->m_next) @@ -3345,34 +3362,39 @@ 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 noexcept { metadata_entry metaentry; return metadata_find(HARD_DISK_METADATA_TAG, 0, metaentry); } -bool chd_file::is_cd() const +std::error_condition chd_file::check_is_cd() const noexcept { metadata_entry metaentry; - return metadata_find(CDROM_OLD_METADATA_TAG, 0, metaentry) - || metadata_find(CDROM_TRACK_METADATA_TAG, 0, metaentry) - || metadata_find(CDROM_TRACK_METADATA2_TAG, 0, metaentry); + std::error_condition err = metadata_find(CDROM_OLD_METADATA_TAG, 0, metaentry); + if (err == error::METADATA_NOT_FOUND) + err = metadata_find(CDROM_TRACK_METADATA_TAG, 0, metaentry); + if (err == error::METADATA_NOT_FOUND) + err = metadata_find(CDROM_TRACK_METADATA2_TAG, 0, metaentry); + return err; } -bool chd_file::is_gd() const +std::error_condition chd_file::check_is_gd() const noexcept { metadata_entry metaentry; - return metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry) - || metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry); + std::error_condition err = metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry); + if (err == error::METADATA_NOT_FOUND) + err = metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry); + return err; } -bool chd_file::is_dvd() const +std::error_condition chd_file::check_is_dvd() const noexcept { metadata_entry metaentry; return metadata_find(DVD_METADATA_TAG, 0, metaentry); } -bool chd_file::is_av() const +std::error_condition chd_file::check_is_av() const noexcept { metadata_entry metaentry; return metadata_find(AV_METADATA_TAG, 0, metaentry); diff --git a/src/lib/util/chd.h b/src/lib/util/chd.h index 7c293f7424e..ecf9ae6fb7d 100644 --- a/src/lib/util/chd.h +++ b/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; - bool is_cd() const; - bool is_gd() const; - bool is_dvd() const; - bool is_av() const; + std::error_condition check_is_hd() const noexcept; + std::error_condition check_is_cd() const noexcept; + std::error_condition check_is_gd() const noexcept; + std::error_condition check_is_dvd() const noexcept; + std::error_condition check_is_av() const noexcept; 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 noexcept; 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); @@ -466,7 +466,7 @@ private: // operations void reset(); - uint64_t find(util::crc16_t crc16, util::sha1_t sha1); + uint64_t find(util::crc16_t crc16, util::sha1_t sha1) const noexcept; void add(uint64_t itemnum, util::crc16_t crc16, util::sha1_t sha1); // constants @@ -563,7 +563,7 @@ private: osd_work_queue * m_read_queue; // work queue for reading uint64_t m_read_queue_offset;// next offset to enqueue uint64_t m_read_done_offset; // next offset that will complete - bool m_read_error; // error during reading? + std::error_condition m_read_error; // error during reading, if any // work item thread static constexpr int WORK_BUFFER_HUNKS = 256; diff --git a/src/lib/util/dvdrom.cpp b/src/lib/util/dvdrom.cpp index 36dc29e7215..0b2379ef7e6 100644 --- a/src/lib/util/dvdrom.cpp +++ b/src/lib/util/dvdrom.cpp @@ -58,8 +58,8 @@ dvdrom_file::dvdrom_file(chd_file *_chd) throw nullptr; /* check it's actually a DVD-ROM */ - if (!chd->is_dvd()) - throw nullptr; + if (std::error_condition err = chd->check_is_dvd()) + throw err; sector_count = chd->unit_count(); } diff --git a/src/tools/chdman.cpp b/src/tools/chdman.cpp index c71ed2a76e8..4b36414e456 100644 --- a/src/tools/chdman.cpp +++ b/src/tools/chdman.cpp @@ -2372,11 +2372,11 @@ static void do_copy(parameters_map ¶ms) // process compression; we default to our current preferences using metadata to pick the type chd_codec_type compression[4]; - if (input_chd.is_hd() || input_chd.is_dvd()) + if (!input_chd.check_is_hd() || !input_chd.check_is_dvd()) parse_compression(params, s_default_hd_compression, output_parent, compression); - else if (input_chd.is_av()) + else if (!input_chd.check_is_av()) parse_compression(params, s_default_ld_compression, output_parent, compression); - else if (input_chd.is_cd() || input_chd.is_gd()) + else if (!input_chd.check_is_cd() || !input_chd.check_is_gd()) parse_compression(params, s_default_cd_compression, output_parent, compression); else parse_compression(params, s_default_raw_compression, output_parent, compression);