Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::unique_ptr rather than raw pointer #3179

Merged
merged 7 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 21 additions & 44 deletions src/tiffcomposite_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,29 +89,6 @@ TiffBinaryArray::TiffBinaryArray(uint16_t tag, IfdId group, const ArraySet* arra
// We'll figure out the correct cfg later
}

TiffDirectory::~TiffDirectory() {
for (auto&& component : components_) {
delete component;
}
delete pNext_;
}

TiffSubIfd::~TiffSubIfd() {
for (auto&& ifd : ifds_) {
delete ifd;
}
}

TiffEntryBase::~TiffEntryBase() {
delete pValue_;
}

TiffBinaryArray::~TiffBinaryArray() {
for (auto&& element : elements_) {
delete element;
}
}

TiffEntryBase::TiffEntryBase(const TiffEntryBase& rhs) :
TiffComponent(rhs),
tiffType_(rhs.tiffType_),
Expand Down Expand Up @@ -228,8 +205,7 @@ void TiffEntryBase::setValue(Value::UniquePtr value) {
return;
tiffType_ = toTiffType(value->typeId());
count_ = value->count();
delete pValue_;
pValue_ = value.release();
pValue_ = std::move(value);
}

void TiffDataEntry::setStrips(const Value* pSize, const byte* pData, size_t sizeData, size_t baseOffset) {
Expand Down Expand Up @@ -446,11 +422,11 @@ TiffComponent* TiffDirectory::doAddPath(uint16_t tag, TiffPath& tiffPath, TiffCo
// condition takes care of them, see below.
if (tiffPath.size() > 1 || (tpi.extendedTag() == 0x927c && tpi.group() == IfdId::exifId)) {
if (tpi.extendedTag() == Tag::next) {
tc = pNext_;
tc = pNext_.get();
} else {
for (auto&& component : components_) {
if (component->tag() == tpi.tag() && component->group() == tpi.group()) {
tc = component;
tc = component.get();
break;
}
}
Expand Down Expand Up @@ -576,14 +552,17 @@ TiffComponent* TiffComponent::doAddChild(UniquePtr /*tiffComponent*/) {
} // TiffComponent::doAddChild

TiffComponent* TiffDirectory::doAddChild(TiffComponent::UniquePtr tiffComponent) {
TiffComponent* tc = tiffComponent.release();
components_.push_back(tc);
return tc;
components_.push_back(std::move(tiffComponent));
return components_.back().get();
} // TiffDirectory::doAddChild

TiffComponent* TiffSubIfd::doAddChild(TiffComponent::UniquePtr tiffComponent) {
auto d = dynamic_cast<TiffDirectory*>(tiffComponent.release());
ifds_.push_back(d);
auto d = dynamic_cast<TiffDirectory*>(tiffComponent.get());
if (!d) {
throw Error(ErrorCode::kerErrorMessage, "dynamic_cast to TiffDirectory failed");
}
tiffComponent.release();
ifds_.push_back(std::unique_ptr<TiffDirectory>(d));
return d;
} // TiffSubIfd::doAddChild

Expand All @@ -600,10 +579,9 @@ TiffComponent* TiffIfdMakernote::doAddChild(TiffComponent::UniquePtr tiffCompone
}

TiffComponent* TiffBinaryArray::doAddChild(TiffComponent::UniquePtr tiffComponent) {
TiffComponent* tc = tiffComponent.release();
elements_.push_back(tc);
elements_.push_back(std::move(tiffComponent));
setDecoded(true);
return tc;
return elements_.back().get();
} // TiffBinaryArray::doAddChild

TiffComponent* TiffComponent::addNext(TiffComponent::UniquePtr tiffComponent) {
Expand All @@ -615,12 +593,11 @@ TiffComponent* TiffComponent::doAddNext(UniquePtr /*tiffComponent*/) {
} // TiffComponent::doAddNext

TiffComponent* TiffDirectory::doAddNext(TiffComponent::UniquePtr tiffComponent) {
TiffComponent* tc = nullptr;
if (hasNext_) {
tc = tiffComponent.release();
pNext_ = tc;
pNext_ = std::move(tiffComponent);
return pNext_.get();
}
return tc;
return nullptr;
} // TiffDirectory::doAddNext

TiffComponent* TiffMnEntry::doAddNext(TiffComponent::UniquePtr tiffComponent) {
Expand Down Expand Up @@ -876,7 +853,7 @@ size_t TiffDirectory::doWrite(IoWrapper& ioWrapper, ByteOrder byteOrder, size_t
idx += 2;
// b) Directory entries - may contain pointers to the value or data
for (auto&& component : components_) {
idx += writeDirEntry(ioWrapper, byteOrder, offset, component, valueIdx, dataIdx, imageIdx);
idx += writeDirEntry(ioWrapper, byteOrder, offset, component.get(), valueIdx, dataIdx, imageIdx);
if (size_t sv = component->size(); sv > 4) {
sv += sv & 1; // Align value to word boundary
valueIdx += sv;
Expand Down Expand Up @@ -1205,15 +1182,15 @@ 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
if (pSubIfd) {
EXV_ERROR << "Multiple sub-IFD image data tags found\n";
}
#endif
pSubIfd = component;
pSubIfd = component.get();
continue;
}
len += component->writeImage(ioWrapper, byteOrder);
Expand Down Expand Up @@ -1502,13 +1479,13 @@ TiffType toTiffType(TypeId typeId) {
return static_cast<uint16_t>(typeId);
}

bool cmpTagLt(const TiffComponent* lhs, const TiffComponent* rhs) {
bool cmpTagLt(const std::unique_ptr<TiffComponent>& lhs, const std::unique_ptr<TiffComponent>& rhs) {
if (lhs->tag() != rhs->tag())
return lhs->tag() < rhs->tag();
return lhs->idx() < rhs->idx();
}

bool cmpGroupLt(const TiffComponent* lhs, const TiffComponent* rhs) {
bool cmpGroupLt(const std::unique_ptr<TiffDirectory>& lhs, const std::unique_ptr<TiffDirectory>& rhs) {
return lhs->group() < rhs->group();
}

Expand Down
22 changes: 11 additions & 11 deletions src/tiffcomposite_int.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<TiffComponent*>;
using Components = std::vector<UniquePtr>;

//! @name Creators
//@{
Expand Down Expand Up @@ -390,7 +390,7 @@ class TiffEntryBase : public TiffComponent {
}

//! Virtual destructor.
~TiffEntryBase() override;
~TiffEntryBase() override {};
//@}

//! @name NOT implemented
Expand Down Expand Up @@ -475,7 +475,7 @@ class TiffEntryBase : public TiffComponent {
}
//! Return a const pointer to the converted value of this component
[[nodiscard]] const Value* pValue() const {
return pValue_;
return pValue_.get();
}
//@}

Expand Down Expand Up @@ -553,7 +553,7 @@ class TiffEntryBase : public TiffComponent {
byte* pData_{}; //!< Pointer to the data area

int idx_{}; //!< Unique id of the entry in the image
Value* pValue_{}; //!< Converted data value
std::unique_ptr<Value> pValue_{}; //!< Converted data value

// This DataBuf is only used when TiffEntryBase::setData is called.
// Otherwise, it remains empty. It is wrapped in a shared_ptr because
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
TiffComponent* pNext_{}; //!< Pointer to the next IFD
UniquePtr pNext_; //!< Pointer to the next IFD
};

/*!
Expand All @@ -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
Expand Down Expand Up @@ -989,7 +989,7 @@ class TiffSubIfd : public TiffEntryBase {

private:
//! A collection of TIFF directories (IFDs)
using Ifds = std::vector<TiffDirectory*>;
using Ifds = std::vector<std::unique_ptr<TiffDirectory>>;

// DATA
IfdId newGroup_; //!< Start of the range of group numbers for the sub-IFDs
Expand Down Expand Up @@ -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;
//@}

Expand Down Expand Up @@ -1470,13 +1470,13 @@ 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 TiffComponent* lhs, const TiffComponent* rhs);
bool cmpTagLt(const TiffComponent::UniquePtr& lhs, const TiffComponent::UniquePtr& rhs);

/*!
@brief Compare two TIFF component pointers by group. Return true if the
group of component lhs is less than that of rhs.
*/
bool cmpGroupLt(const TiffComponent* lhs, const TiffComponent* rhs);
bool cmpGroupLt(const std::unique_ptr<TiffDirectory>& lhs, const std::unique_ptr<TiffDirectory>& rhs);

//! Function to create and initialize a new TIFF entry
TiffComponent::UniquePtr newTiffEntry(uint16_t tag, IfdId group);
Expand Down
4 changes: 2 additions & 2 deletions src/tiffvisitor_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ 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_) {
p += updateDirEntry(p, byteOrder(), component);
for (const auto& component : object->components_) {
p += updateDirEntry(p, byteOrder(), component.get());
}
}

Expand Down
Loading