From 2bf89f4854d2e10b5192644b3dea944798603fee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Tue, 24 Apr 2018 23:59:26 +0200 Subject: [PATCH 1/3] Store pointers to Impl classes in auto_ptr Pimpl class Impl is stored in raw pointers which are allocated on the heap in the constructor. However, the constructor can throw an exception resulting in a memory leak as the destructor is **not** invoked. => A smart pointer is however properly deallocated. --- include/exiv2/basicio.hpp | 4 ++-- include/exiv2/properties.hpp | 2 +- include/exiv2/tags.hpp | 2 +- include/exiv2/xmp_exiv2.hpp | 2 +- src/basicio.cpp | 2 -- src/properties.cpp | 1 - src/tags.cpp | 5 +---- src/xmp.cpp | 1 - 8 files changed, 6 insertions(+), 13 deletions(-) diff --git a/include/exiv2/basicio.hpp b/include/exiv2/basicio.hpp index 619a3561..b851a768 100644 --- a/include/exiv2/basicio.hpp +++ b/include/exiv2/basicio.hpp @@ -526,7 +526,7 @@ namespace Exiv2 { // Pimpl idiom class Impl; - Impl* p_; + std::auto_ptr p_; }; // class FileIo @@ -726,7 +726,7 @@ namespace Exiv2 { // Pimpl idiom class Impl; - Impl* p_; + std::auto_ptr p_; }; // class MemIo diff --git a/include/exiv2/properties.hpp b/include/exiv2/properties.hpp index 41576f0a..40eeb64a 100644 --- a/include/exiv2/properties.hpp +++ b/include/exiv2/properties.hpp @@ -312,7 +312,7 @@ namespace Exiv2 { private: // Pimpl idiom struct Impl; - Impl* p_; + std::auto_ptr p_; }; // class XmpKey diff --git a/include/exiv2/tags.hpp b/include/exiv2/tags.hpp index b9585046..4a3f33e4 100644 --- a/include/exiv2/tags.hpp +++ b/include/exiv2/tags.hpp @@ -222,7 +222,7 @@ namespace Exiv2 { private: // Pimpl idiom struct Impl; - Impl* p_; + std::auto_ptr p_; }; // class ExifKey diff --git a/include/exiv2/xmp_exiv2.hpp b/include/exiv2/xmp_exiv2.hpp index 6ccab6f5..1c51fdd9 100644 --- a/include/exiv2/xmp_exiv2.hpp +++ b/include/exiv2/xmp_exiv2.hpp @@ -160,7 +160,7 @@ namespace Exiv2 { private: // Pimpl idiom struct Impl; - Impl* p_; + std::auto_ptr p_; }; // class Xmpdatum diff --git a/src/basicio.cpp b/src/basicio.cpp index 985f6d10..415d8e94 100644 --- a/src/basicio.cpp +++ b/src/basicio.cpp @@ -371,7 +371,6 @@ namespace Exiv2 { FileIo::~FileIo() { close(); - delete p_; } int FileIo::munmap() @@ -1141,7 +1140,6 @@ namespace Exiv2 { if (p_->isMalloced_) { std::free(p_->data_); } - delete p_; } long MemIo::write(const byte* data, long wcount) diff --git a/src/properties.cpp b/src/properties.cpp index a933f64c..898fb7f8 100644 --- a/src/properties.cpp +++ b/src/properties.cpp @@ -2740,7 +2740,6 @@ namespace Exiv2 { XmpKey::~XmpKey() { - delete p_; } XmpKey::XmpKey(const XmpKey& rhs) : Key(rhs), p_(new Impl(*rhs.p_)) diff --git a/src/tags.cpp b/src/tags.cpp index ab22e577..9e74c990 100644 --- a/src/tags.cpp +++ b/src/tags.cpp @@ -3168,10 +3168,7 @@ namespace Exiv2 { { } - ExifKey::~ExifKey() - { - delete p_; - } + ExifKey::~ExifKey() {} ExifKey& ExifKey::operator=(const ExifKey& rhs) { diff --git a/src/xmp.cpp b/src/xmp.cpp index f149d3f7..e9c72145 100644 --- a/src/xmp.cpp +++ b/src/xmp.cpp @@ -175,7 +175,6 @@ namespace Exiv2 { Xmpdatum::~Xmpdatum() { - delete p_; } std::string Xmpdatum::key() const From fc60d97a294e8a7dc7cc3c0a4c3101ceadcff073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Wed, 25 Apr 2018 00:02:08 +0200 Subject: [PATCH 2/3] Prevent passing of null-pointers to libc functions --- src/basicio.cpp | 12 ++++++++---- src/tiffcomposite_int.cpp | 4 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/basicio.cpp b/src/basicio.cpp index 415d8e94..e887d444 100644 --- a/src/basicio.cpp +++ b/src/basicio.cpp @@ -1091,9 +1091,9 @@ namespace Exiv2 { void MemIo::Impl::reserve(long wcount) { - long need = wcount + idx_; + const long need = wcount + idx_; long blockSize = 32*1024; // 32768 ` - long maxBlockSize = 4*1024*1024; + const long maxBlockSize = 4*1024*1024; if (!isMalloced_) { // Minimum size for 1st block @@ -1102,7 +1102,9 @@ namespace Exiv2 { if ( data == NULL ) { throw Error(kerMallocFailed); } - std::memcpy(data, data_, size_); + if (data_ != NULL) { + std::memcpy(data, data_, size_); + } data_ = data; sizeAlloced_ = size; isMalloced_ = true; @@ -1146,7 +1148,9 @@ namespace Exiv2 { { p_->reserve(wcount); assert(p_->isMalloced_); - std::memcpy(&p_->data_[p_->idx_], data, wcount); + if (data != NULL) { + std::memcpy(&p_->data_[p_->idx_], data, wcount); + } p_->idx_ += wcount; return wcount; } diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp index f95dfbe9..1db9ac71 100644 --- a/src/tiffcomposite_int.cpp +++ b/src/tiffcomposite_int.cpp @@ -380,7 +380,9 @@ namespace Exiv2 { if (newSize > size_) { setData(DataBuf(newSize)); } - memset(pData_, 0x0, size_); + if (pData_ != NULL) { + memset(pData_, 0x0, size_); + } size_ = value->copy(pData_, byteOrder); assert(size_ == newSize); setValue(value); From b1a31352b63e43214d94e238232737197f847d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Wed, 25 Apr 2018 00:06:40 +0200 Subject: [PATCH 3/3] Add default constructor for XmpData --- include/exiv2/xmp_exiv2.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/exiv2/xmp_exiv2.hpp b/include/exiv2/xmp_exiv2.hpp index 1c51fdd9..b503bc7e 100644 --- a/include/exiv2/xmp_exiv2.hpp +++ b/include/exiv2/xmp_exiv2.hpp @@ -179,6 +179,9 @@ namespace Exiv2 { */ class EXIV2API XmpData { public: + //! Default constructor + XmpData() : xmpMetadata_(), xmpPacket_(), usePacket_(0) {} + //! XmpMetadata iterator type typedef XmpMetadata::iterator iterator; //! XmpMetadata const iterator type