From bf7cfafb3ed6b1fad24bcda4e0e460266009aa49 Mon Sep 17 00:00:00 2001 From: Vladislav Kalinin Date: Sun, 10 Nov 2024 02:53:57 +0100 Subject: [PATCH] Pointers in `sections` and `directories` classes should be under smart pointers --- coffi/coffi.hpp | 94 ++++++++++++++-------------- coffi/coffi_directory.hpp | 42 ++++--------- coffi/coffi_section.hpp | 68 ++++++++------------ coffi/coffi_utils.hpp | 127 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 118 deletions(-) diff --git a/coffi/coffi.hpp b/coffi/coffi.hpp index 9806070..b838423 100644 --- a/coffi/coffi.hpp +++ b/coffi/coffi.hpp @@ -411,7 +411,7 @@ class coffi : public coffi_strings, } coff_header_->set_optional_header_size(narrow_cast( optional_header_->get_sizeof() + win_header_->get_sizeof() + - directories_.size() * sizeof(image_data_directory))); + directories_.get_count() * sizeof(image_data_directory))); } //--------------------------------------------------------------------- @@ -485,17 +485,18 @@ class coffi : public coffi_strings, */ section* add_section(const std::string& name) { - section* sec; + std::unique_ptr
sec; if (architecture_ == COFFI_ARCHITECTURE_TI) { - sec = new section_impl_ti{this, this, this}; + sec = std::make_unique(this, this, this); } else { - sec = new section_impl{this, this, this}; + sec = std::make_unique(this, this, this); } - sec->set_index(narrow_cast(sections_.size())); + sec->set_index(narrow_cast(sections_.get_count())); sec->set_name(name); - sections_.push_back(sec); - return sec; + section* sec_ptr = sec.get(); + sections_.append(std::move(sec)); + return sec_ptr; } //--------------------------------------------------------------------- @@ -522,12 +523,13 @@ class coffi : public coffi_strings, */ directory* add_directory(const image_data_directory& rva_and_size) { - directory* d = - new directory(narrow_cast(directories_.size())); + std::unique_ptr d = + std::make_unique(narrow_cast(directories_.get_count())); d->set_virtual_address(rva_and_size.virtual_address); d->set_size(rva_and_size.size); - directories_.push_back(d); - return d; + directory* d_ptr = d.get(); + directories_.append(std::move(d)); + return d_ptr; } //--------------------------------------------------------------------- @@ -658,7 +660,7 @@ class coffi : public coffi_strings, return false; } sec->set_index(i); - sections_.push_back(sec.release()); + sections_.append(std::move(sec)); } return true; @@ -676,7 +678,7 @@ class coffi : public coffi_strings, // Compute the header fields coff_header_->set_sections_count( - narrow_cast(sections_.size())); + narrow_cast(sections_.get_count())); if (symbols_.size() > 0) { coff_header_->set_symbols_count( symbols_.back().get_index() + @@ -695,8 +697,8 @@ class coffi : public coffi_strings, uint32_t size_of_initialized_data = 0; uint32_t size_of_uninitialized_data = 0; for (const auto §ion : sections_) { - const uint32_t flags = section->get_flags(); - const uint32_t data_size = section->get_data_size(); + const uint32_t flags = section.get_flags(); + const uint32_t data_size = section.get_data_size(); if (flags & IMAGE_SCN_CNT_CODE) { size_of_code += data_size; } @@ -714,7 +716,7 @@ class coffi : public coffi_strings, } if (win_header_) { win_header_->set_number_of_rva_and_sizes( - narrow_cast(directories_.size())); + narrow_cast(directories_.get_count())); coff_header_->set_optional_header_size(narrow_cast( coff_header_->get_optional_header_size() + win_header_->get_sizeof() + directories_.get_sizeof())); @@ -727,13 +729,13 @@ class coffi : public coffi_strings, coff_header_->get_sizeof() + coff_header_->get_optional_header_size(); for (const auto& section : sections_) { - size_of_headers += section->get_sizeof(); + size_of_headers += section.get_sizeof(); } size_of_headers = alignTo(size_of_headers, file_alignment); uint32_t size_of_image = alignTo(size_of_headers, section_alignment); for (const auto §ion : sections_) { - const uint32_t virtual_size = section->get_virtual_size(); + const uint32_t virtual_size = section.get_virtual_size(); if (virtual_size) { size_of_image += alignTo(virtual_size, section_alignment); } @@ -761,8 +763,8 @@ class coffi : public coffi_strings, } } - for (auto sec : sections_) { - sec->save_header(stream); + for (auto &sec : sections_) { + sec.save_header(stream); } for (auto dp : data_pages_) { @@ -784,7 +786,7 @@ class coffi : public coffi_strings, directories_[dp.index]->save_data(stream); break; case DATA_PAGE_UNUSED: - stream.write(unused_spaces_[dp.index].data, + stream.write(unused_spaces_[dp.index].data.get(), unused_spaces_[dp.index].size); break; } @@ -843,29 +845,29 @@ class coffi : public coffi_strings, void populate_data_pages() { data_pages_.clear(); - for (auto sec : sections_) { - if (sec->get_data_offset() > 0 || sec->get_data()) { + for (auto &sec : sections_) { + if (sec.get_data_offset() > 0 || sec.get_data()) { data_pages_.push_back( - data_page{DATA_PAGE_RAW, sec->get_data_offset(), - sec->get_data_size(), sec->get_index()}); + data_page{DATA_PAGE_RAW, sec.get_data_offset(), + sec.get_data_size(), sec.get_index()}); } - if (sec->get_reloc_offset() > 0 || sec->get_reloc_count() > 0) { + if (sec.get_reloc_offset() > 0 || sec.get_reloc_count() > 0) { data_pages_.push_back(data_page{ - DATA_PAGE_RELOCATIONS, sec->get_reloc_offset(), - sec->get_relocations_filesize(), sec->get_index()}); + DATA_PAGE_RELOCATIONS, sec.get_reloc_offset(), + sec.get_relocations_filesize(), sec.get_index()}); } if ((architecture_ != COFFI_ARCHITECTURE_TI) && - (sec->get_line_num_count() > 0)) { + (sec.get_line_num_count() > 0)) { data_pages_.push_back(data_page{ - DATA_PAGE_LINE_NUMBERS, sec->get_line_num_offset(), - sec->get_line_numbers_filesize(), sec->get_index()}); + DATA_PAGE_LINE_NUMBERS, sec.get_line_num_offset(), + sec.get_line_numbers_filesize(), sec.get_index()}); } } - for (auto d : directories_) { - if (d->get_data_filesize() > 0) { + for (auto& d : directories_) { + if (d.get_data_filesize() > 0) { data_pages_.push_back(data_page{DATA_PAGE_DIRECTORY, - d->get_virtual_address(), - d->get_size(), d->get_index()}); + d.get_virtual_address(), + d.get_size(), d.get_index()}); } } @@ -905,8 +907,8 @@ class coffi : public coffi_strings, offset += narrow_cast(win_header_->get_sizeof()); } offset += directories_.get_sizeof(); - for (auto sec : sections_) { - offset += narrow_cast(sec->get_sizeof()); + for (auto &sec : sections_) { + offset += narrow_cast(sec.get_sizeof()); } return offset; } @@ -982,10 +984,10 @@ class coffi : public coffi_strings, file_alignment - (previous_size % file_alignment); if (previous_dp && previous_dp->type == DATA_PAGE_RAW) { // Extend the previous section data - char* padding = new char[size]; + std::unique_ptr padding = std::make_unique(size); if (padding) { - std::memset(padding, 0, size); - sections_[previous_dp->index]->append_data(padding, + std::memset(padding.get(), 0, size); + sections_[previous_dp->index]->append_data(padding.get(), size); } } @@ -1005,8 +1007,8 @@ class coffi : public coffi_strings, //--------------------------------------------------------------------- void clean_unused_spaces() { - for (auto us : unused_spaces_) { - delete[] us.data; + for (auto& us : unused_spaces_) { + us.data.reset(); } unused_spaces_.clear(); } @@ -1016,12 +1018,12 @@ class coffi : public coffi_strings, add_unused_space(uint32_t offset, uint32_t size, uint8_t padding_byte = 0) { unused_space us; - us.data = new char[size]; + us.data = std::make_unique(size); if (us.data) { - std::memset(us.data, padding_byte, size); + std::memset(us.data.get(), padding_byte, size); us.size = size; us.offset = offset; - unused_spaces_.push_back(us); + unused_spaces_.emplace_back(std::move(us)); } } @@ -1050,7 +1052,7 @@ class coffi : public coffi_strings, { uint32_t offset; uint32_t size; - char* data; + std::unique_ptr data; }; //--------------------------------------------------------------------- diff --git a/coffi/coffi_directory.hpp b/coffi/coffi_directory.hpp index 968e12a..4f60774 100644 --- a/coffi/coffi_directory.hpp +++ b/coffi/coffi_directory.hpp @@ -169,29 +169,17 @@ class directory }; /*! @brief List of image data directories - * - * It is implemented as a vector of @ref directory pointers. - */ -class directories : public std::vector + * + * It is implemented as a vector of @ref directory pointers. + */ +class directories : public unique_ptr_collection { public: - //------------------------------------------------------------------------------ - directories(sections_provider* scn) : scn_{scn} {} + explicit directories(sections_provider* scn) : scn_{scn} {} //! Discards the copy constructor directories(const directories&) = delete; - virtual ~directories() { clean(); } - - void clean() - { - for (auto d : *this) { - delete d; - } - clear(); - } - - //------------------------------------------------------------------------------ bool load(std::istream& stream) { for (uint32_t i = 0; @@ -200,42 +188,38 @@ class directories : public std::vector if (!d->load(stream)) { return false; } - push_back(d.release()); + append(std::move(d)); } return true; } - //------------------------------------------------------------------------------ bool load_data(std::istream& stream) { - for (auto d : *this) { - if (!d->load_data(stream)) { + for (auto& d : *this) { + if (!d.load_data(stream)) { return false; } } return true; } - //--------------------------------------------------------------------- void save(std::ostream& stream) const { - for (auto d : *this) { - d->save(stream); + for (const auto& d : *this) { + d.save(stream); } } - //------------------------------------------------------------------------------ uint32_t get_sizeof() const { - if (size() > 0) { - return narrow_cast(size() * (*begin())->get_sizeof()); + if (get_count() > 0) { + return narrow_cast(get_count() * (*begin()).get_sizeof()); } + return 0; } - //------------------------------------------------------------------------------ protected: - //------------------------------------------------------------------------------ sections_provider* scn_; }; diff --git a/coffi/coffi_section.hpp b/coffi/coffi_section.hpp index 09ee373..ab37493 100644 --- a/coffi/coffi_section.hpp +++ b/coffi/coffi_section.hpp @@ -457,75 +457,57 @@ class section_impl_ti : public section_impl_tmpl }; /*! @brief List of sections - * - * It is implemented as a vector of @ref section pointers. - * This allows to manage easily all the different section implementations (for every COFF format), - * with pointers to their base interface class (@ref section). - */ -//------------------------------------------------------------------------------ -class sections : public std::vector + * + * It is implemented as a vector of @ref section pointers. + * This allows to manage easily all the different section implementations (for every COFF format), + * with pointers to their base interface class (@ref section). + */ +class sections : public unique_ptr_collection
{ public: - //------------------------------------------------------------------------------ - sections() {} + sections() = default; - //------------------------------------------------------------------------------ //! Discards the copy constructor sections(const sections&) = delete; - //------------------------------------------------------------------------------ - virtual ~sections() { clean(); } + sections(sections&&) = default; - //------------------------------------------------------------------------------ - void clean() - { - for (auto sec : *this) { - delete sec; - } - clear(); - } - - //------------------------------------------------------------------------------ /*! @brief Subscript operator, finds a section by its index - * - * @param[in] i Index of the section - * @return A reference to the element at specified location **i** - */ + * + * @param[in] i Index of the section + * @return A reference to the element at specified location **i** + */ section* operator[](size_t i) { - return std::vector::operator[](i); + return items_[i].get(); } - //------------------------------------------------------------------------------ - /*! @copydoc operator[](size_t) - */ + //! @copydoc operator[](size_t) const section* operator[](size_t i) const { - return std::vector::operator[](i); + return items_[i].get(); } - //------------------------------------------------------------------------------ /*! @brief Subscript operator, finds a section by its symbolic name - * - * @param[in] name Symbolic name of the section - * @return A reference to the element with the section symbolic name **name** - */ + * + * @param[in] name Symbolic name of the section + * @return A reference to the element with the section symbolic name **name** + */ section* operator[](const std::string& name) { return (section*)((const sections*)this)->operator[](name); } - //------------------------------------------------------------------------------ - /*! @copydoc operator[](const std::string&) - */ + //! @copydoc operator[](const std::string&) const section* operator[](const std::string& name) const { - for (section* sec : *this) { - if (sec->get_name() == name) { - return sec; + for (auto& section : items_) { + if (section->get_name() == name) { + return section.get(); } } - return 0; + + return nullptr; } }; diff --git a/coffi/coffi_utils.hpp b/coffi/coffi_utils.hpp index 4e6a65e..0ce1066 100644 --- a/coffi/coffi_utils.hpp +++ b/coffi/coffi_utils.hpp @@ -110,4 +110,131 @@ THE SOFTWARE. //! @} +template class unique_ptr_collection +{ + public: + class iterator + { + public: + using iterator_category = std::input_iterator_tag; + using difference_type = std::ptrdiff_t; + using value_type = T; + using pointer = value_type*; + using reference = value_type&; + + explicit iterator( + typename std::vector>::iterator&& iterator) + : iterator_(std::move(iterator)) + { + } + + reference operator*() const { return *iterator_->get(); } + + pointer operator->() { return iterator_->get(); } + + iterator& operator++() + { + ++iterator_; + return *this; + } + + iterator operator++(int) + { + iterator tmp = *this; + iterator_++; + return tmp; + } + + friend bool operator==(const iterator& lhs, const iterator& rhs) + { + return lhs.iterator_ == rhs.iterator_; + } + + friend bool operator!=(const iterator& lhs, const iterator& rhs) + { + return lhs.iterator_ != rhs.iterator_; + } + + private: + typename std::vector>::iterator iterator_; + }; + + class const_iterator + { + public: + using iterator_category = std::input_iterator_tag; + using difference_type = std::ptrdiff_t; + using value_type = T; + using pointer = const value_type*; + using reference = const value_type&; + + explicit const_iterator( + typename std::vector>::const_iterator && iterator) + : iterator_(std::move(iterator)) + { + } + + reference operator*() const { return *iterator_->get(); } + + pointer operator->() { return iterator_->get(); } + + const_iterator& operator++() + { + ++iterator_; + return *this; + } + + const_iterator operator++(int) + { + iterator tmp = *this; + iterator_++; + return tmp; + } + + friend bool operator==(const const_iterator& lhs, const const_iterator& rhs) + { + return lhs.iterator_ == rhs.iterator_; + } + + friend bool operator!=(const const_iterator& lhs, const const_iterator& rhs) + { + return lhs.iterator_ != rhs.iterator_; + } + + private: + typename std::vector>::const_iterator iterator_; + }; + + T* operator[](size_t i) + { + return items_[i].get(); + } + + const T* operator[](size_t i) const + { + return items_[i].get(); + } + + void clean() { items_.clear(); } + + [[nodiscard]] size_t get_count() const { return items_.size(); } + + void append(std::unique_ptr&& item) + { + items_.emplace_back(std::move(item)); + } + + iterator begin() { return iterator(items_.begin()); } + iterator end() { return iterator(items_.end()); } + + const_iterator begin() const { return const_iterator(items_.begin()); } + const_iterator end() const { return const_iterator(items_.end()); } + + const_iterator cbegin() const { return begin(); } + const_iterator cend() const { return end(); } + + protected: + std::vector> items_; +}; + #endif // COFFI_UTILS_HPP