Code review suggestions from @neheb.

Exiv2 · Feb 19, 2025 · 3be906d · 3be906d
1 parent 50052da
commit 3be906d
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 23 deletions.
diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp
@@ -89,14 +89,6 @@ TiffBinaryArray::TiffBinaryArray(uint16_t tag, IfdId group, const ArraySet* arra
   // We'll figure out the correct cfg later
 }
 
-TiffDirectory::~TiffDirectory() {}
-
-TiffSubIfd::~TiffSubIfd() {}
-
-TiffEntryBase::~TiffEntryBase() {}
-
-TiffBinaryArray::~TiffBinaryArray() {}
-
 TiffEntryBase::TiffEntryBase(const TiffEntryBase& rhs) :
     TiffComponent(rhs),
     tiffType_(rhs.tiffType_),
@@ -213,7 +205,7 @@ void TiffEntryBase::setValue(Value::UniquePtr value) {
     return;
   tiffType_ = toTiffType(value->typeId());
   count_ = value->count();
-  std::swap(pValue_, value);
+  pValue_ = std::move(value);
 }
 
 void TiffDataEntry::setStrips(const Value* pSize, const byte* pData, size_t sizeData, size_t baseOffset) {
@@ -560,9 +552,8 @@ TiffComponent* TiffComponent::doAddChild(UniquePtr /*tiffComponent*/) {
 }  // TiffComponent::doAddChild
 
 TiffComponent* TiffDirectory::doAddChild(TiffComponent::UniquePtr tiffComponent) {
-  TiffComponent* tc = tiffComponent.get();
   components_.push_back(std::move(tiffComponent));
-  return tc;
+  return components_.back().get();
 }  // TiffDirectory::doAddChild
 
 TiffComponent* TiffSubIfd::doAddChild(TiffComponent::UniquePtr tiffComponent) {
@@ -588,10 +579,9 @@ TiffComponent* TiffIfdMakernote::doAddChild(TiffComponent::UniquePtr tiffCompone
 }
 
 TiffComponent* TiffBinaryArray::doAddChild(TiffComponent::UniquePtr tiffComponent) {
-  TiffComponent* tc = tiffComponent.get();
   elements_.push_back(std::move(tiffComponent));
   setDecoded(true);
-  return tc;
+  return elements_.back().get();
 }  // TiffBinaryArray::doAddChild
 
 TiffComponent* TiffComponent::addNext(TiffComponent::UniquePtr tiffComponent) {
@@ -604,7 +594,7 @@ TiffComponent* TiffComponent::doAddNext(UniquePtr /*tiffComponent*/) {
 
 TiffComponent* TiffDirectory::doAddNext(TiffComponent::UniquePtr tiffComponent) {
   if (hasNext_) {
-    std::swap(pNext_, tiffComponent);
+    pNext_ = std::move(tiffComponent);
     return pNext_.get();
   }
   return nullptr;
@@ -1192,7 +1182,7 @@ size_t TiffComponent::writeImage(IoWrapper& ioWrapper, ByteOrder byteOrder) cons
 size_t TiffDirectory::doWriteImage(IoWrapper& ioWrapper, ByteOrder byteOrder) const {
   size_t len = 0;
   TiffComponent* pSubIfd = nullptr;
-  for (auto& component : components_) {
+  for (const auto& component : components_) {
     if (component->tag() == 0x014a) {
       // Hack: delay writing of sub-IFD image data to get the order correct
 #ifndef SUPPRESS_WARNINGS

diff --git a/src/tiffcomposite_int.hpp b/src/tiffcomposite_int.hpp
@@ -154,7 +154,7 @@ class TiffComponent {
   //! TiffComponent auto_ptr type
   using UniquePtr = std::unique_ptr<TiffComponent>;
   //! Container type to hold all metadata
-  using Components = std::vector<std::unique_ptr<TiffComponent>>;
+  using Components = std::vector<UniquePtr>;
 
   //! @name Creators
   //@{
@@ -390,7 +390,7 @@ class TiffEntryBase : public TiffComponent {
   }
 
   //! Virtual destructor.
-  ~TiffEntryBase() override;
+  ~TiffEntryBase() override {};
   //@}
 
   //! @name NOT implemented
@@ -830,7 +830,7 @@ class TiffDirectory : public TiffComponent {
   //! Default constructor
   TiffDirectory(uint16_t tag, IfdId group, bool hasNext = true);
   //! Virtual destructor
-  ~TiffDirectory() override;
+  ~TiffDirectory() override {};
   //@}
 
   //! @name NOT implemented
@@ -919,7 +919,7 @@ class TiffDirectory : public TiffComponent {
   // DATA
   Components components_;   //!< List of components in this directory
   bool hasNext_;            //!< True if the directory has a next pointer
-  UniquePtr pNext_{};       //!< Pointer to the next IFD
+  UniquePtr pNext_;         //!< Pointer to the next IFD
 };
 
 /*!
@@ -938,7 +938,7 @@ class TiffSubIfd : public TiffEntryBase {
   //! Default constructor
   TiffSubIfd(uint16_t tag, IfdId group, IfdId newGroup);
   //! Virtual destructor
-  ~TiffSubIfd() override;
+  ~TiffSubIfd() override {};
   //@}
 
   //! @name Protected Creators
@@ -1269,7 +1269,7 @@ class TiffBinaryArray : public TiffEntryBase {
   //! Constructor for a complex binary array
   TiffBinaryArray(uint16_t tag, IfdId group, const ArraySet* arraySet, size_t setSize, CfgSelFct cfgSelFct);
   //! Virtual destructor
-  ~TiffBinaryArray() override;
+  ~TiffBinaryArray() override {};
   TiffBinaryArray& operator=(const TiffBinaryArray&) = delete;
   //@}
 
@@ -1470,7 +1470,7 @@ class TiffBinaryElement : public TiffEntryBase {
   @brief Compare two TIFF component pointers by tag. Return true if the tag
          of component lhs is less than that of rhs.
  */
-bool cmpTagLt(const std::unique_ptr<TiffComponent>& lhs, const std::unique_ptr<TiffComponent>& rhs);
+bool cmpTagLt(const TiffComponent::UniquePtr& lhs, const TiffComponent::UniquePtr& rhs);
 
 /*!
   @brief Compare two TIFF component pointers by group. Return true if the

diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp
@@ -580,7 +580,7 @@ void TiffEncoder::visitDirectory(TiffDirectory* /*object*/) {
 void TiffEncoder::visitDirectoryNext(TiffDirectory* object) {
   // Update type and count in IFD entries, in case they changed
   byte* p = object->start() + 2;
-  for (auto& component : object->components_) {
+  for (const auto& component : object->components_) {
     p += updateDirEntry(p, byteOrder(), component.get());
   }
 }