From 998d6a77c37bb9c6d05924fdb8d48b488238b1a9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Sep 2019 21:40:49 -0400 Subject: [PATCH 1/5] General: Remove redundant inline keyword Functions defined within the class declaration are already inline by default, so we don't need to specify the keyword here. --- include/nod/DiscBase.hpp | 80 ++++++++++++++++++++-------------------- include/nod/Util.hpp | 16 ++++---- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/include/nod/DiscBase.hpp b/include/nod/DiscBase.hpp index ef791d0..a2d80b2 100644 --- a/include/nod/DiscBase.hpp +++ b/include/nod/DiscBase.hpp @@ -35,10 +35,10 @@ public: offset = SBig(off); length = SBig(len); } - inline bool isDir() const { return ((SBig(typeAndNameOffset) >> 24) != 0); } - inline uint32_t getNameOffset() const { return SBig(typeAndNameOffset) & 0xffffff; } - inline uint32_t getOffset() const { return SBig(offset); } - inline uint32_t getLength() const { return SBig(length); } + bool isDir() const { return ((SBig(typeAndNameOffset) >> 24) != 0); } + uint32_t getNameOffset() const { return SBig(typeAndNameOffset) & 0xffffff; } + uint32_t getOffset() const { return SBig(offset); } + uint32_t getLength() const { return SBig(length); } void incrementLength() { uint32_t orig = SBig(length); ++orig; @@ -187,13 +187,13 @@ private: public: Node(const IPartition& parent, const FSTNode& node, std::string_view name); - inline Kind getKind() const { return m_kind; } - inline std::string_view getName() const { return m_name; } - inline uint64_t size() const { return m_discLength; } + Kind getKind() const { return m_kind; } + std::string_view getName() const { return m_name; } + uint64_t size() const { return m_discLength; } std::unique_ptr beginReadStream(uint64_t offset = 0) const; std::unique_ptr getBuf() const; - inline std::vector::iterator rawBegin() const { return m_childrenBegin; } - inline std::vector::iterator rawEnd() const { return m_childrenEnd; } + std::vector::iterator rawBegin() const { return m_childrenBegin; } + std::vector::iterator rawEnd() const { return m_childrenEnd; } class DirectoryIterator { friend class Node; @@ -207,21 +207,21 @@ public: using pointer = Node*; using reference = Node&; - inline bool operator!=(const DirectoryIterator& other) { return m_it != other.m_it; } - inline bool operator==(const DirectoryIterator& other) { return m_it == other.m_it; } - inline DirectoryIterator& operator++() { + bool operator!=(const DirectoryIterator& other) { return m_it != other.m_it; } + bool operator==(const DirectoryIterator& other) { return m_it == other.m_it; } + DirectoryIterator& operator++() { if (m_it->m_kind == Kind::Directory) m_it = m_it->rawEnd(); else ++m_it; return *this; } - inline Node& operator*() { return *m_it; } - inline Node* operator->() { return &*m_it; } + Node& operator*() { return *m_it; } + Node* operator->() { return &*m_it; } }; - inline DirectoryIterator begin() const { return DirectoryIterator(m_childrenBegin); } - inline DirectoryIterator end() const { return DirectoryIterator(m_childrenEnd); } - inline DirectoryIterator find(std::string_view name) const { + DirectoryIterator begin() const { return DirectoryIterator(m_childrenBegin); } + DirectoryIterator end() const { return DirectoryIterator(m_childrenEnd); } + DirectoryIterator find(std::string_view name) const { if (m_kind == Kind::Directory) { DirectoryIterator it = begin(); for (; it != end(); ++it) { @@ -289,47 +289,47 @@ public: IPartition(const DiscBase& parent, PartitionKind kind, bool isWii, uint64_t offset) : m_parent(parent), m_kind(kind), m_offset(offset), m_isWii(isWii) {} virtual uint64_t normalizeOffset(uint64_t anOffset) const { return anOffset; } - inline PartitionKind getKind() const { return m_kind; } - inline bool isWii() const { return m_isWii; } - inline uint64_t getDiscOffset() const { return m_offset; } + PartitionKind getKind() const { return m_kind; } + bool isWii() const { return m_isWii; } + uint64_t getDiscOffset() const { return m_offset; } virtual std::unique_ptr beginReadStream(uint64_t offset = 0) const = 0; - inline std::unique_ptr beginDOLReadStream(uint64_t offset = 0) const { + std::unique_ptr beginDOLReadStream(uint64_t offset = 0) const { return beginReadStream(m_dolOff + offset); } - inline std::unique_ptr beginFSTReadStream(uint64_t offset = 0) const { + std::unique_ptr beginFSTReadStream(uint64_t offset = 0) const { return beginReadStream(m_fstOff + offset); } - inline std::unique_ptr beginApploaderReadStream(uint64_t offset = 0) const { + std::unique_ptr beginApploaderReadStream(uint64_t offset = 0) const { return beginReadStream(0x2440 + offset); } - inline const Node& getFSTRoot() const { return m_nodes[0]; } - inline Node& getFSTRoot() { return m_nodes[0]; } + const Node& getFSTRoot() const { return m_nodes[0]; } + Node& getFSTRoot() { return m_nodes[0]; } bool extractToDirectory(SystemStringView path, const ExtractionContext& ctx); - inline uint64_t getDOLSize() const { return m_dolSz; } - inline std::unique_ptr getDOLBuf() const { + uint64_t getDOLSize() const { return m_dolSz; } + std::unique_ptr getDOLBuf() const { std::unique_ptr buf(new uint8_t[m_dolSz]); beginDOLReadStream()->read(buf.get(), m_dolSz); return buf; } - inline uint64_t getFSTSize() const { return m_fstSz; } - inline std::unique_ptr getFSTBuf() const { + uint64_t getFSTSize() const { return m_fstSz; } + std::unique_ptr getFSTBuf() const { std::unique_ptr buf(new uint8_t[m_fstSz]); beginFSTReadStream()->read(buf.get(), m_fstSz); return buf; } - inline uint64_t getApploaderSize() const { return m_apploaderSz; } - inline std::unique_ptr getApploaderBuf() const { + uint64_t getApploaderSize() const { return m_apploaderSz; } + std::unique_ptr getApploaderBuf() const { std::unique_ptr buf(new uint8_t[m_apploaderSz]); beginApploaderReadStream()->read(buf.get(), m_apploaderSz); return buf; } - inline size_t getNodeCount() const { return m_nodes.size(); } - inline const Header& getHeader() const { return m_header; } - inline const BI2Header& getBI2() const { return m_bi2Header; } + size_t getNodeCount() const { return m_nodes.size(); } + const Header& getHeader() const { return m_header; } + const BI2Header& getBI2() const { return m_bi2Header; } virtual bool extractCryptoFiles(SystemStringView path, const ExtractionContext& ctx) const { return true; } bool extractSysFiles(SystemStringView path, const ExtractionContext& ctx) const; }; @@ -346,29 +346,29 @@ protected: public: DiscBase(std::unique_ptr&& dio, bool& err) : m_discIO(std::move(dio)), m_header(*m_discIO, err) {} - inline const Header& getHeader() const { return m_header; } - inline const IDiscIO& getDiscIO() const { return *m_discIO; } - inline size_t getPartitonNodeCount(size_t partition = 0) const { + const Header& getHeader() const { return m_header; } + const IDiscIO& getDiscIO() const { return *m_discIO; } + size_t getPartitonNodeCount(size_t partition = 0) const { if (partition > m_partitions.size()) return -1; return m_partitions[partition]->getNodeCount(); } - inline IPartition* getDataPartition() { + IPartition* getDataPartition() { for (const std::unique_ptr& part : m_partitions) if (part->getKind() == PartitionKind::Data) return part.get(); return nullptr; } - inline IPartition* getUpdatePartition() { + IPartition* getUpdatePartition() { for (const std::unique_ptr& part : m_partitions) if (part->getKind() == PartitionKind::Update) return part.get(); return nullptr; } - inline void extractToDirectory(SystemStringView path, const ExtractionContext& ctx) { + void extractToDirectory(SystemStringView path, const ExtractionContext& ctx) { for (std::unique_ptr& part : m_partitions) part->extractToDirectory(path, ctx); } diff --git a/include/nod/Util.hpp b/include/nod/Util.hpp index 490f2b4..82e2df5 100644 --- a/include/nod/Util.hpp +++ b/include/nod/Util.hpp @@ -94,8 +94,8 @@ public: m_utf8.assign(len, '\0'); WideCharToMultiByte(CP_UTF8, 0, str.data(), str.size(), &m_utf8[0], len, nullptr, nullptr); } - inline std::string_view utf8_str() const { return m_utf8; } - inline const char* c_str() const { return m_utf8.c_str(); } + std::string_view utf8_str() const { return m_utf8; } + const char* c_str() const { return m_utf8.c_str(); } }; class SystemStringConv { std::wstring m_sys; @@ -106,8 +106,8 @@ public: m_sys.assign(len, L'\0'); MultiByteToWideChar(CP_UTF8, 0, str.data(), str.size(), &m_sys[0], len); } - inline SystemStringView sys_str() const { return m_sys; } - inline const SystemChar* c_str() const { return m_sys.c_str(); } + SystemStringView sys_str() const { return m_sys; } + const SystemChar* c_str() const { return m_sys.c_str(); } }; #ifndef _SYS_STR #define _SYS_STR(val) L##val @@ -124,16 +124,16 @@ class SystemUTF8Conv { public: explicit SystemUTF8Conv(SystemStringView str) : m_utf8(str) {} - inline std::string_view utf8_str() const { return m_utf8; } - inline const char* c_str() const { return m_utf8.data(); } + std::string_view utf8_str() const { return m_utf8; } + const char* c_str() const { return m_utf8.data(); } }; class SystemStringConv { SystemStringView m_sys; public: explicit SystemStringConv(std::string_view str) : m_sys(str) {} - inline SystemStringView sys_str() const { return m_sys; } - inline const SystemChar* c_str() const { return m_sys.data(); } + SystemStringView sys_str() const { return m_sys; } + const SystemChar* c_str() const { return m_sys.data(); } }; #ifndef _SYS_STR #define _SYS_STR(val) val From f443b60bdeec6de869f06d10402cad65e4a599eb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Sep 2019 21:51:17 -0400 Subject: [PATCH 2/5] DiscBase: Mark member functions as const where applicable These don't modify instance state, so they can be marked as const. --- include/nod/DiscBase.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/nod/DiscBase.hpp b/include/nod/DiscBase.hpp index a2d80b2..4448217 100644 --- a/include/nod/DiscBase.hpp +++ b/include/nod/DiscBase.hpp @@ -207,8 +207,8 @@ public: using pointer = Node*; using reference = Node&; - bool operator!=(const DirectoryIterator& other) { return m_it != other.m_it; } - bool operator==(const DirectoryIterator& other) { return m_it == other.m_it; } + bool operator==(const DirectoryIterator& other) const { return m_it == other.m_it; } + bool operator!=(const DirectoryIterator& other) const { return !operator==(other); } DirectoryIterator& operator++() { if (m_it->m_kind == Kind::Directory) m_it = m_it->rawEnd(); @@ -217,7 +217,9 @@ public: return *this; } Node& operator*() { return *m_it; } + const Node& operator*() const { return *m_it; } Node* operator->() { return &*m_it; } + const Node* operator->() const { return &*m_it; } }; DirectoryIterator begin() const { return DirectoryIterator(m_childrenBegin); } DirectoryIterator end() const { return DirectoryIterator(m_childrenEnd); } From 4bba7af2c2cc2c6e0a457e6b98c6fe3fb304d099 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Sep 2019 21:52:40 -0400 Subject: [PATCH 3/5] DiscBase: std::move std::function instance std::function isn't a trivial type (it's allowed to heap allocate to store any necessary captures), so we can move it in the constructor to avoid any unnecessary allocations --- include/nod/DiscBase.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/nod/DiscBase.hpp b/include/nod/DiscBase.hpp index 4448217..87ac776 100644 --- a/include/nod/DiscBase.hpp +++ b/include/nod/DiscBase.hpp @@ -457,7 +457,7 @@ public: : m_outPath(outPath) , m_fileIO(NewFileIO(outPath, discCapacity)) , m_discCapacity(discCapacity) - , m_progressCB(progressCB) {} + , m_progressCB(std::move(progressCB)) {} DiscBuilderBase(DiscBuilderBase&&) = default; DiscBuilderBase& operator=(DiscBuilderBase&&) = default; From 7ddff919c1b020cc729b49db67f820fea033c09c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Sep 2019 21:55:56 -0400 Subject: [PATCH 4/5] DiscBase: Prevent potential off-by-one case within getPartitonNodeCount() We should be comparing with >= instead of >. --- include/nod/DiscBase.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/nod/DiscBase.hpp b/include/nod/DiscBase.hpp index 87ac776..f5d54be 100644 --- a/include/nod/DiscBase.hpp +++ b/include/nod/DiscBase.hpp @@ -351,8 +351,9 @@ public: const Header& getHeader() const { return m_header; } const IDiscIO& getDiscIO() const { return *m_discIO; } size_t getPartitonNodeCount(size_t partition = 0) const { - if (partition > m_partitions.size()) + if (partition >= m_partitions.size()) { return -1; + } return m_partitions[partition]->getNodeCount(); } From f5c3cbdcd7321e9335f2a10c4df6f53b3af3c999 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Sep 2019 21:57:41 -0400 Subject: [PATCH 5/5] DiscBase: Amend typo within getPartitonNodeCount() name Adds an extra 'i' to correct a typo. --- include/nod/DiscBase.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/nod/DiscBase.hpp b/include/nod/DiscBase.hpp index f5d54be..b666f25 100644 --- a/include/nod/DiscBase.hpp +++ b/include/nod/DiscBase.hpp @@ -350,7 +350,7 @@ public: const Header& getHeader() const { return m_header; } const IDiscIO& getDiscIO() const { return *m_discIO; } - size_t getPartitonNodeCount(size_t partition = 0) const { + size_t getPartitionNodeCount(size_t partition = 0) const { if (partition >= m_partitions.size()) { return -1; }