Fix leak in kemoticons
Needs ReviewPublic

Authored by kfunk on Mon, Dec 3, 7:28 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

ASAN trace:
Indirect leak of 64 byte(s) in 2 object(s) allocated from:

#0 0x52a000 in operator new(unsigned long) (/home/kfunk/devel/install/kf5/bin/kmail+0x52a000)
#1 0x7fbac897e192 in QList<KEmoticonsProvider::Emoticon>::node_construct(QList<KEmoticonsProvider::Emoticon>::Node*, KEmoticonsProvider::Emoticon const&) /home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qlist.h:435:65
#2 0x7fbac897d184 in QList<KEmoticonsProvider::Emoticon>::append(KEmoticonsProvider::Emoticon const&) /home/kfunk/devel/build/qt5.11/qtbase/include/QtCore/../../../../../src/qt5.11/qtbase/src/corelib/tools/qlist.h:584:13
#3 0x7fbac897c59c in KEmoticonsProvider::addIndexItem(QString const&, QStringList const&) /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticonsprovider.cpp:162:39
#4 0x7fba8a15f7b5 in KdeEmoticons::loadTheme(QString const&) /home/kfunk/devel/src/kf5/kemoticons/src/providers/kde/kde_emoticons.cpp:183:13
#5 0x7fbac896ceb7 in KEmoticonsPrivate::loadTheme(QString const&)::$_6::operator()(QString const&, QString const&, KEmoticonsProvider*) const /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:126:19
#6 0x7fbac896cca1 in KEmoticonsPrivate::loadTheme(QString const&) /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:151:24
#7 0x7fbac896d24f in KEmoticons::theme(QString const&) const /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:181:15
#8 0x7fbac896d1ab in KEmoticons::theme() const /home/kfunk/devel/src/kf5/kemoticons/src/core/kemoticons.cpp:171:12
#9 0x7fba8a2e25bb in KTextToHTMLEmoticons::parseEmoticons(QString const&, bool, QStringList const&) /home/kfunk/devel/src/kf5/kemoticons/src/integrationplugin/ktexttohtml.cpp:43:24
...

Diff Detail

Repository
R301 KEmoticons
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5670
Build 5688: arc lint + arc unit
kfunk created this revision.Mon, Dec 3, 7:28 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMon, Dec 3, 7:28 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
kfunk requested review of this revision.Mon, Dec 3, 7:28 AM
kfunk added a comment.Mon, Dec 3, 7:29 AM

Not entirely sure about whether this is the right fix for this. Can someone check why this was commented before?

The leak is fixed with this patch. Unit tests still work, too.

And it kind of makes sense to have the KEmoticonsTheme class own its provider.

anthonyfieroni added inline comments.
src/core/kemoticonstheme.cpp
32–33

Make it shared pointer, KEmoticonsTheme::KEmoticonsTheme(const KEmoticonsTheme &ket) initialize provider with raw other pointer, deletion will cause a segfault in use after.

41–42

Remove that line.

kfunk updated this revision to Diff 46787.Mon, Dec 3, 2:13 PM

Use QSharedPointer

anthonyfieroni added inline comments.Mon, Dec 3, 3:15 PM
src/core/kemoticonstheme.cpp
57

I've rethink it, this line can be a problem if pointer is not owned by us. Did you know consumer of the KEmoticonsTheme?