From 901a68e2e0bb0d9178ffdb59e128718c1495250f Mon Sep 17 00:00:00 2001 From: AJR Date: Sun, 22 Oct 2023 16:04:04 -0400 Subject: chd.cpp, chdcodec.cpp: Minor refactoring - Return std::error_condition from set_raw_sha1 and set_parent_sha1 instead of throwing exceptions - Fix a few cases where error codes could be swallowed - Catch exceptions in is_XXX predicates - Add const qualifier to SHA-1 extraction methods - Add noexcept qualifier to a few internal functions - Clean up various comments --- src/lib/util/chd.cpp | 173 ++++++++++++++++++++++++++++++++-------------- src/lib/util/chd.h | 16 ++--- src/lib/util/chdcodec.cpp | 6 +- src/lib/util/chdcodec.h | 4 +- src/tools/chdman.cpp | 8 ++- 5 files changed, 139 insertions(+), 68 deletions(-) diff --git a/src/lib/util/chd.cpp b/src/lib/util/chd.cpp index 11f0b93eb86..ba96e3897a5 100644 --- a/src/lib/util/chd.cpp +++ b/src/lib/util/chd.cpp @@ -134,7 +134,7 @@ struct chd_file::metadata_hash // stream in bigendian order //------------------------------------------------- -inline util::sha1_t chd_file::be_read_sha1(const uint8_t *base)const +inline util::sha1_t chd_file::be_read_sha1(const uint8_t *base) const noexcept { util::sha1_t result; memcpy(&result.m_raw[0], base, sizeof(result.m_raw)); @@ -147,7 +147,7 @@ inline util::sha1_t chd_file::be_read_sha1(const uint8_t *base)const // stream in bigendian order //------------------------------------------------- -inline void chd_file::be_write_sha1(uint8_t *base, util::sha1_t value) +inline void chd_file::be_write_sha1(uint8_t *base, util::sha1_t value) noexcept { memcpy(base, &value.m_raw[0], sizeof(value.m_raw)); } @@ -338,7 +338,7 @@ bool chd_file::parent_missing() const noexcept * @return A sha1_t. */ -util::sha1_t chd_file::sha1() +util::sha1_t chd_file::sha1() const { try { @@ -349,7 +349,7 @@ util::sha1_t chd_file::sha1() } catch (std::error_condition const &) { - // on failure, return nullptr + // on failure, return null return util::sha1_t::null; } } @@ -367,7 +367,7 @@ util::sha1_t chd_file::sha1() * @return A sha1_t. */ -util::sha1_t chd_file::raw_sha1() +util::sha1_t chd_file::raw_sha1() const { try { @@ -382,7 +382,7 @@ util::sha1_t chd_file::raw_sha1() } catch (std::error_condition const &) { - // on failure, return nullptr + // on failure, return null return util::sha1_t::null; } } @@ -400,7 +400,7 @@ util::sha1_t chd_file::raw_sha1() * @return A sha1_t. */ -util::sha1_t chd_file::parent_sha1() +util::sha1_t chd_file::parent_sha1() const { try { @@ -415,7 +415,7 @@ util::sha1_t chd_file::parent_sha1() } catch (std::error_condition const &) { - // on failure, return nullptr + // on failure, return null return util::sha1_t::null; } } @@ -532,33 +532,45 @@ std::error_condition chd_file::hunk_info(uint32_t hunknum, chd_codec_type &compr } /** - * @fn void chd_file::set_raw_sha1(sha1_t rawdata) + * @fn std::error_condition chd_file::set_raw_sha1(sha1_t rawdata) * * @brief ------------------------------------------------- * set_raw_sha1 - set our SHA1 values * -------------------------------------------------. * * @param rawdata The rawdata. + * + * @return A std::error_condition. */ -void chd_file::set_raw_sha1(util::sha1_t rawdata) +std::error_condition chd_file::set_raw_sha1(util::sha1_t rawdata) { - // create a big-endian version - uint8_t rawbuf[sizeof(util::sha1_t)]; - be_write_sha1(rawbuf, rawdata); + // wrap this for clean reporting + try + { + // create a big-endian version + uint8_t rawbuf[sizeof(util::sha1_t)]; + be_write_sha1(rawbuf, rawdata); - // write to the header - uint64_t offset = (m_rawsha1_offset != 0) ? m_rawsha1_offset : m_sha1_offset; - assert(offset != 0); - file_write(offset, rawbuf, sizeof(rawbuf)); + // write to the header + uint64_t offset = (m_rawsha1_offset != 0) ? m_rawsha1_offset : m_sha1_offset; + assert(offset != 0); + file_write(offset, rawbuf, sizeof(rawbuf)); - // if we have a separate rawsha1_offset, update the full sha1 as well - if (m_rawsha1_offset != 0) - metadata_update_hash(); + // if we have a separate rawsha1_offset, update the full sha1 as well + if (m_rawsha1_offset != 0) + metadata_update_hash(); + return std::error_condition(); + } + catch (std::error_condition const &err) + { + // return any errors + return err; + } } /** - * @fn void chd_file::set_parent_sha1(sha1_t parent) + * @fn std::error_condition chd_file::set_parent_sha1(sha1_t parent) * * @brief ------------------------------------------------- * set_parent_sha1 - set the parent SHA1 value @@ -567,21 +579,33 @@ void chd_file::set_raw_sha1(util::sha1_t rawdata) * @exception CHDERR_INVALID_FILE Thrown when a chderr invalid file error condition occurs. * * @param parent The parent. + * + * @return A std::error_condition. */ -void chd_file::set_parent_sha1(util::sha1_t parent) +std::error_condition chd_file::set_parent_sha1(util::sha1_t parent) { // if no file, fail if (!m_file) - throw std::error_condition(error::INVALID_FILE); + return error::INVALID_FILE; - // create a big-endian version - uint8_t rawbuf[sizeof(util::sha1_t)]; - be_write_sha1(rawbuf, parent); + // wrap this for clean reporting + try + { + // create a big-endian version + uint8_t rawbuf[sizeof(util::sha1_t)]; + be_write_sha1(rawbuf, parent); - // write to the header - assert(m_parentsha1_offset != 0); - file_write(m_parentsha1_offset, rawbuf, sizeof(rawbuf)); + // write to the header + assert(m_parentsha1_offset != 0); + file_write(m_parentsha1_offset, rawbuf, sizeof(rawbuf)); + return std::error_condition(); + } + catch (std::error_condition const &err) + { + // return any errors + return err; + } } /** @@ -893,7 +917,7 @@ void chd_file::close() * @param hunknum The hunknum. * @param [in,out] buffer If non-null, the buffer. * - * @return The hunk. + * @return A std::error_condition. */ std::error_condition chd_file::read_hunk(uint32_t hunknum, void *buffer) @@ -970,10 +994,12 @@ std::error_condition chd_file::read_hunk(uint32_t hunknum, void *buffer) else if (m_parent_missing) throw std::error_condition(error::REQUIRES_PARENT); else if (m_parent) - m_parent->read_hunk(hunknum, dest); + return m_parent->read_hunk(hunknum, dest); else + { memset(dest, 0, m_hunkbytes); - return std::error_condition(); + return std::error_condition(); + } } // compressed case @@ -2597,7 +2623,7 @@ void chd_file::hunk_copy_from_self(uint32_t hunknum, uint32_t otherhunk) // only permitted to reference prior hunks if (otherhunk >= hunknum) - throw std::error_condition(std::errc::invalid_argument); + throw std::error_condition(error::HUNK_OUT_OF_RANGE); // update the map entry uint8_t *rawmap = &m_rawmap[hunknum * 12]; @@ -2943,8 +2969,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; } @@ -2997,10 +3023,14 @@ std::error_condition chd_file_compressor::compress_continue(double &progress, do else { osd_work_queue_wait(m_read_queue, 30 * osd_ticks_per_second()); - if (!compressed()) - return std::error_condition(); - set_raw_sha1(m_compsha1.finish()); - return compress_v5_map(); + std::error_condition err; + if (compressed()) + { + err = set_raw_sha1(m_compsha1.finish()); + if (!err) + err = compress_v5_map(); + } + return err; } } } @@ -3165,7 +3195,9 @@ void chd_file_compressor::async_read() uint8_t *curdest = dest; for (uint64_t curoffs = m_read_done_offset; curoffs < end_offset + 1; curoffs += hunk_bytes()) { - m_parent->read_hunk(curoffs / hunk_bytes(), curdest); + std::error_condition err = m_parent->read_hunk(curoffs / hunk_bytes(), curdest); + if (err) + throw err; curdest += hunk_bytes(); } } @@ -3314,34 +3346,69 @@ void chd_file_compressor::hashmap::add(uint64_t itemnum, util::crc16_t crc16, ut bool chd_file::is_hd() const { - metadata_entry metaentry; - return metadata_find(HARD_DISK_METADATA_TAG, 0, metaentry); + 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 { - 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); + try + { + 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); + } + catch (std::error_condition const &) + { + return false; + } } bool chd_file::is_gd() const { - metadata_entry metaentry; - return metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry) - || metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry); + try + { + metadata_entry metaentry; + return metadata_find(GDROM_OLD_METADATA_TAG, 0, metaentry) + || metadata_find(GDROM_TRACK_METADATA_TAG, 0, metaentry); + } + catch (std::error_condition const &) + { + return false; + } } bool chd_file::is_dvd() const { - metadata_entry metaentry; - return metadata_find(DVD_METADATA_TAG, 0, metaentry); + 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 { - metadata_entry metaentry; - return metadata_find(AV_METADATA_TAG, 0, metaentry); + try + { + metadata_entry metaentry; + return metadata_find(AV_METADATA_TAG, 0, metaentry); + } + catch (std::error_condition const &) + { + return false; + } } diff --git a/src/lib/util/chd.h b/src/lib/util/chd.h index 3e52cab4f9b..be9f94aa341 100644 --- a/src/lib/util/chd.h +++ b/src/lib/util/chd.h @@ -309,18 +309,18 @@ public: uint32_t hunk_count() const noexcept { return m_hunkcount; } uint32_t unit_bytes() const noexcept { return m_unitbytes; } uint64_t unit_count() const noexcept { return m_unitcount; } - bool compressed() const { return (m_compression[0] != CHD_CODEC_NONE); } + bool compressed() const noexcept { return (m_compression[0] != CHD_CODEC_NONE); } chd_codec_type compression(int index) const noexcept { return m_compression[index]; } chd_file *parent() const noexcept { return m_parent.get(); } bool parent_missing() const noexcept; - util::sha1_t sha1(); - util::sha1_t raw_sha1(); - util::sha1_t parent_sha1(); + util::sha1_t sha1() const; + util::sha1_t raw_sha1() const; + util::sha1_t parent_sha1() const; std::error_condition hunk_info(uint32_t hunknum, chd_codec_type &compressor, uint32_t &compbytes); // setters - void set_raw_sha1(util::sha1_t rawdata); - void set_parent_sha1(util::sha1_t parent); + std::error_condition set_raw_sha1(util::sha1_t rawdata); + std::error_condition set_parent_sha1(util::sha1_t parent); // file create std::error_condition create(std::string_view filename, uint64_t logicalbytes, uint32_t hunkbytes, uint32_t unitbytes, chd_codec_type compression[4]); @@ -372,8 +372,8 @@ private: struct metadata_hash; // inline helpers - util::sha1_t be_read_sha1(const uint8_t *base) const; - void be_write_sha1(uint8_t *base, util::sha1_t value); + util::sha1_t be_read_sha1(const uint8_t *base) const noexcept; + void be_write_sha1(uint8_t *base, util::sha1_t value) noexcept; void file_read(uint64_t offset, void *dest, uint32_t length) const; void file_write(uint64_t offset, const void *source, uint32_t length); uint64_t file_append(const void *source, uint32_t length, uint32_t alignment = 0); diff --git a/src/lib/util/chdcodec.cpp b/src/lib/util/chdcodec.cpp index fa46975285e..41465fd0614 100644 --- a/src/lib/util/chdcodec.cpp +++ b/src/lib/util/chdcodec.cpp @@ -512,7 +512,7 @@ const codec_entry f_codec_list[] = // instance of the given type //------------------------------------------------- -const codec_entry *find_in_list(chd_codec_type type) +const codec_entry *find_in_list(chd_codec_type type) noexcept { // find in the list and construct the class for (auto & elem : f_codec_list) @@ -627,7 +627,7 @@ chd_decompressor::ptr chd_codec_list::new_decompressor(chd_codec_type type, chd_ // corresponds to a supported codec //------------------------------------------------- -bool chd_codec_list::codec_exists(chd_codec_type type) +bool chd_codec_list::codec_exists(chd_codec_type type) noexcept { // find in the list and construct the class return bool(find_in_list(type)); @@ -639,7 +639,7 @@ bool chd_codec_list::codec_exists(chd_codec_type type) // codec //------------------------------------------------- -const char *chd_codec_list::codec_name(chd_codec_type type) +const char *chd_codec_list::codec_name(chd_codec_type type) noexcept { // find in the list and construct the class const codec_entry *entry = find_in_list(type); diff --git a/src/lib/util/chdcodec.h b/src/lib/util/chdcodec.h index 700ae8f8979..3fbc8a0896e 100644 --- a/src/lib/util/chdcodec.h +++ b/src/lib/util/chdcodec.h @@ -104,8 +104,8 @@ public: static chd_decompressor::ptr new_decompressor(chd_codec_type type, chd_file &file); // utilities - static bool codec_exists(chd_codec_type type); - static const char *codec_name(chd_codec_type type); + static bool codec_exists(chd_codec_type type) noexcept; + static const char *codec_name(chd_codec_type type) noexcept; }; diff --git a/src/tools/chdman.cpp b/src/tools/chdman.cpp index 3a4277c50d1..39098a8820a 100644 --- a/src/tools/chdman.cpp +++ b/src/tools/chdman.cpp @@ -1660,7 +1660,9 @@ static void do_verify(parameters_map ¶ms) // fix it if requested; this also fixes the overall one so we don't need to do any more if (params.find(OPTION_FIX) != params.end()) { - input_chd.set_raw_sha1(computed_sha1); + std::error_condition err = input_chd.set_raw_sha1(computed_sha1); + if (err) + report_error(1, "Error updating SHA-1: %s", err.message()); printf("SHA-1 updated to correct value in input CHD\n"); } } @@ -1682,7 +1684,9 @@ static void do_verify(parameters_map ¶ms) // fix it if requested if (params.find(OPTION_FIX) != params.end()) { - input_chd.set_raw_sha1(computed_sha1); + std::error_condition err = input_chd.set_raw_sha1(computed_sha1); + if (err) + report_error(1, "Error updating SHA-1: %s", err.message()); printf("SHA-1 updated to correct value in input CHD\n"); } } -- cgit v1.2.3