Skip to content

Commit

Permalink
get rid of -fanalyzer memory leaks
Browse files Browse the repository at this point in the history
Don't use make_shared inside a function. Instead, change constructor to
value to have std::move.

Also move shared_ptrs everywhere. It's fairly expensive to copy.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
  • Loading branch information
neheb committed Jan 3, 2023
1 parent d458bf2 commit f981c51
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 13 deletions.
18 changes: 10 additions & 8 deletions src/tiffcomposite_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,16 +212,16 @@ int TiffEntryBase::idx() const {
return idx_;
}

void TiffEntryBase::setData(const std::shared_ptr<DataBuf>& buf) {
storage_ = buf;
pData_ = buf->data();
size_ = buf->size();
void TiffEntryBase::setData(std::shared_ptr<DataBuf> buf) {
storage_ = std::move(buf);
pData_ = storage_->data();
size_ = storage_->size();
}

void TiffEntryBase::setData(byte* pData, size_t size, const std::shared_ptr<DataBuf>& storage) {
void TiffEntryBase::setData(byte* pData, size_t size, std::shared_ptr<DataBuf> storage) {
pData_ = pData;
size_ = size;
storage_ = storage;
storage_ = std::move(storage);
if (!pData_)
size_ = 0;
}
Expand All @@ -231,7 +231,8 @@ void TiffEntryBase::updateValue(Value::UniquePtr value, ByteOrder byteOrder) {
return;
size_t newSize = value->size();
if (newSize > size_) {
setData(std::make_shared<DataBuf>(newSize));
auto d = std::make_shared<DataBuf>(newSize);
setData(std::move(d));
}
if (pData_) {
memset(pData_, 0x0, size_);
Expand Down Expand Up @@ -431,7 +432,8 @@ size_t TiffBinaryArray::addElement(size_t idx, const ArrayDef& def) {
// The assertion typically fails if a component is not configured in
// the TIFF structure table (TiffCreator::tiffTreeStruct_)
tp->setStart(pData() + idx);
tp->setData(const_cast<byte*>(pData() + idx), sz, storage());
auto s = storage();
tp->setData(const_cast<byte*>(pData() + idx), sz, std::move(s));
tp->setElDef(def);
tp->setElByteOrder(cfg()->byteOrder_);
addChild(std::move(tc));
Expand Down
6 changes: 3 additions & 3 deletions src/tiffcomposite_int.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,13 @@ class TiffEntryBase : public TiffComponent {
you should pass std::shared_ptr<DataBuf>(), which is essentially
a nullptr.
*/
void setData(byte* pData, size_t size, const std::shared_ptr<DataBuf>& storage);
void setData(byte* pData, size_t size, std::shared_ptr<DataBuf> storage);
/*!
@brief Set the entry's data buffer. A shared_ptr is used to manage the DataBuf
because TiffEntryBase has a clone method so it is possible (in theory) for
the DataBuf to have multiple owners.
*/
void setData(const std::shared_ptr<DataBuf>& buf);
void setData(std::shared_ptr<DataBuf> buf);
/*!
@brief Update the value. Takes ownership of the pointer passed in.
Expand Down Expand Up @@ -534,7 +534,7 @@ class TiffEntryBase : public TiffComponent {
static size_t writeOffset(byte* buf, size_t offset, TiffType tiffType, ByteOrder byteOrder);

//! Used (internally) to create another reference to the DataBuf reference by storage_.
[[nodiscard]] const std::shared_ptr<DataBuf>& storage() const {
[[nodiscard]] std::shared_ptr<DataBuf> storage() const {
return storage_;
}

Expand Down
5 changes: 3 additions & 2 deletions src/tiffvisitor_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,8 @@ void TiffReader::readTiffEntry(TiffEntryBase* object) {
v->read(pData, size, byteOrder());

object->setValue(std::move(v));
object->setData(pData, size, std::make_shared<DataBuf>());
auto d = std::make_shared<DataBuf>();
object->setData(pData, size, std::move(d));
object->setOffset(offset);
object->setIdx(nextIdx(object->group()));
} catch (std::overflow_error&) {
Expand Down Expand Up @@ -1374,7 +1375,7 @@ void TiffReader::visitBinaryArray(TiffBinaryArray* object) {
size_t size = object->TiffEntryBase::doSize();
auto buf = std::make_shared<DataBuf>(cryptFct(object->tag(), pData, size, pRoot_));
if (!buf->empty())
object->setData(buf);
object->setData(std::move(buf));
}

const ArrayDef* defs = object->def();
Expand Down

0 comments on commit f981c51

Please sign in to comment.