From c52de598b2b536182be84faf40f066a2fd5c71b8 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Sat, 17 Oct 2020 17:22:32 +0200 Subject: [PATCH] Fix memory leak in KateMessageLayout Use a vector-of-values instead of a vector-of-pointers for no good reason. This is faster, uses less memory, and prevents leaks like this one: ``` ================================================================= ==175053==ERROR: LeakSanitizer: detected memory leaks Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x7f9f407caf41 in operator new(unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x7f9f405d2dc4 in KateMessageLayout::add(QLayoutItem*, KTextEditor::Message::MessagePosition) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateviewhelpers.cpp:145 #2 0x7f9f405d2815 in KateMessageLayout::addWidget(QWidget*, KTextEditor::Message::MessagePosition) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateviewhelpers.cpp:87 #3 0x7f9f405b3f20 in KTextEditor::ViewPrivate::postMessage(KTextEditor::Message*, QList >) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateview.cpp:3673 #4 0x7f9f404dface in KTextEditor::DocumentPrivate::postMessage(KTextEditor::Message*) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/document/katedocument.cpp:5911 #5 0x55df19c6d620 in MessageTest::testAutoHide() /home/milian/projects/kf5/src/frameworks/ktexteditor/autotests/src/messagetest.cpp:77 #6 0x7ffd039e7bbf ([stack]+0x1fbbf) #7 0x55df19c68e82 in MessageTest::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) autotests/messagetest_autogen/UVLADIE3JM/moc_messagetest.cpp:85 Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x7f9f407caf41 in operator new(unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x7f9f405d2dc4 in KateMessageLayout::add(QLayoutItem*, KTextEditor::Message::MessagePosition) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateviewhelpers.cpp:145 #2 0x7f9f405d2815 in KateMessageLayout::addWidget(QWidget*, KTextEditor::Message::MessagePosition) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateviewhelpers.cpp:87 #3 0x7f9f405b3f20 in KTextEditor::ViewPrivate::postMessage(KTextEditor::Message*, QList >) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/view/kateview.cpp:3673 #4 0x7f9f404dface in KTextEditor::DocumentPrivate::postMessage(KTextEditor::Message*) /home/milian/projects/kf5/src/frameworks/ktexteditor/src/document/katedocument.cpp:5911 #5 0x55df19c6a8a2 in MessageTest::testPostMessage() /home/milian/projects/kf5/src/frameworks/ktexteditor/autotests/src/messagetest.cpp:44 #6 0x7ffd039e7bbf ([stack]+0x1fbbf) ``` --- src/view/kateviewhelpers.cpp | 16 ++++++---------- src/view/kateviewhelpers.h | 5 +++-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/view/kateviewhelpers.cpp b/src/view/kateviewhelpers.cpp index ffc7dadf1..1174c5e08 100644 --- a/src/view/kateviewhelpers.cpp +++ b/src/view/kateviewhelpers.cpp @@ -94,10 +94,7 @@ int KateMessageLayout::count() const QLayoutItem *KateMessageLayout::itemAt(int index) const { - if (index < 0 || index >= m_items.size()) - return nullptr; - - return m_items[index]->item; + return m_items.value(index).item; } void KateMessageLayout::setGeometry(const QRect &rect) @@ -106,9 +103,9 @@ void KateMessageLayout::setGeometry(const QRect &rect) const int s = spacing(); const QRect adjustedRect = rect.adjusted(s, s, -s, -s); - for (auto wrapper : m_items) { - QLayoutItem *item = wrapper->item; - auto position = wrapper->position; + for (const auto &wrapper : qAsConst(m_items)) { + QLayoutItem *item = wrapper.item; + auto position = wrapper.position; if (position == KTextEditor::Message::TopInView) { const QRect r(adjustedRect.width() - item->sizeHint().width(), s, item->sizeHint().width(), item->sizeHint().height()); @@ -134,15 +131,14 @@ QSize KateMessageLayout::sizeHint() const QLayoutItem *KateMessageLayout::takeAt(int index) { if (index >= 0 && index < m_items.size()) { - ItemWrapper *layoutStruct = m_items.takeAt(index); - return layoutStruct->item; + return m_items.takeAt(index).item; } return nullptr; } void KateMessageLayout::add(QLayoutItem *item, KTextEditor::Message::MessagePosition pos) { - m_items.push_back(new ItemWrapper(item, pos)); + m_items.push_back({item, pos}); } // END KateMessageLayout diff --git a/src/view/kateviewhelpers.h b/src/view/kateviewhelpers.h index 06b82f271..f4f505356 100644 --- a/src/view/kateviewhelpers.h +++ b/src/view/kateviewhelpers.h @@ -83,6 +83,7 @@ private: void addItem(QLayoutItem *item) override; // never called publically struct ItemWrapper { + ItemWrapper() = default; ItemWrapper(QLayoutItem *i, KTextEditor::Message::MessagePosition pos) : item(i) , position(pos) @@ -90,10 +91,10 @@ private: } QLayoutItem *item = nullptr; - KTextEditor::Message::MessagePosition position; + KTextEditor::Message::MessagePosition position = KTextEditor::Message::AboveView; }; - QVector m_items; + QVector m_items; }; /** -- GitLab