Fix leak in kemoticons
Needs ReviewPublic
Actions

Authored by kfunk on Dec 3 2018, 7:28 AM.

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.Dec 3 2018, 7:28 AM

Restricted Application added a project: Frameworks. · View Herald TranscriptDec 3 2018, 7:28 AM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

kfunk requested review of this revision.Dec 3 2018, 7:28 AM

Harbormaster completed remote builds in B5661: Diff 46765.Dec 3 2018, 7:28 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 a subscriber: anthonyfieroni.Dec 3 2018, 8:17 AM

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.

Use QSharedPointer

Harbormaster completed remote builds in B5670: Diff 46787.Dec 3 2018, 2:13 PM

anthonyfieroni added inline comments.Dec 3 2018, 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?

Revision Contents
Changeset List

			Path	Packages
M			src/core/kemoticonstheme.cpp (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	46765	e818913		Dec 3 2018, 7:28 AM	★	★
Diff 2	46787	e818913	Use QSharedPointer	Dec 3 2018, 2:13 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
54b0606c567f	3434a653d143	e818913aabf8	Kevin Funk	Fix leak in kemoticons (Show More…)	Dec 3 2018, 7:28 AM

Diff 46787

View Options

src/core/kemoticonstheme.cpp