From bdee748a6a0bbd8b9bdb461fc205177b5ed28675 Mon Sep 17 00:00:00 2001 From: Carlo Teubner <435950+c4rlo@users.noreply.github.com> Date: Sat, 1 Dec 2018 21:08:55 +0000 Subject: [PATCH] Make Entry::m_tmpHistoryItem a QScopedPointer (#2524) * Make m_tmpHistoryItem a QScopedPointer Most of the time, `m_tmpHistoryItem` should be null by the time an `Entry` is destroyed. However, if a caller ever calls `beginUpdate()` without later calling `endUpdate()` -- perhaps because an exception was throw in the meantime -- it may not be null. This change avoids a memory leak in that case. Found via https://lgtm.com/projects/g/keepassxreboot/keepassxc/alerts --- src/core/Entry.cpp | 13 +++++-------- src/core/Entry.h | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 55f56f583..9eb25c278 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -40,7 +40,6 @@ Entry::Entry() , m_attachments(new EntryAttachments(this)) , m_autoTypeAssociations(new AutoTypeAssociations(this)) , m_customData(new CustomData(this)) - , m_tmpHistoryItem(nullptr) , m_modifiedSinceBegin(false) , m_updateTimeinfo(true) { @@ -706,9 +705,9 @@ void Entry::copyDataFrom(const Entry* other) void Entry::beginUpdate() { - Q_ASSERT(!m_tmpHistoryItem); + Q_ASSERT(m_tmpHistoryItem.isNull()); - m_tmpHistoryItem = new Entry(); + m_tmpHistoryItem.reset(new Entry()); m_tmpHistoryItem->setUpdateTimeinfo(false); m_tmpHistoryItem->m_uuid = m_uuid; m_tmpHistoryItem->m_data = m_data; @@ -721,16 +720,14 @@ void Entry::beginUpdate() bool Entry::endUpdate() { - Q_ASSERT(m_tmpHistoryItem); + Q_ASSERT(!m_tmpHistoryItem.isNull()); if (m_modifiedSinceBegin) { m_tmpHistoryItem->setUpdateTimeinfo(true); - addHistoryItem(m_tmpHistoryItem); + addHistoryItem(m_tmpHistoryItem.take()); truncateHistory(); - } else { - delete m_tmpHistoryItem; } - m_tmpHistoryItem = nullptr; + m_tmpHistoryItem.reset(); return m_modifiedSinceBegin; } diff --git a/src/core/Entry.h b/src/core/Entry.h index 655cc3621..ae11abe4d 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -249,7 +249,7 @@ private: QPointer m_customData; QList m_history; // Items sorted from oldest to newest - Entry* m_tmpHistoryItem; + QScopedPointer m_tmpHistoryItem; bool m_modifiedSinceBegin; QPointer m_group; bool m_updateTimeinfo;