KUriFilter: simplify data structures, fix memory leak
ClosedPublic
Actions

Authored by dfaure on Dec 2 2017, 9:36 AM.

Details

Reviewers

apol
davidedmundson
arichardson
bshah

Commits

R241:73fd5f771cc4: KUriFilter: simplify data structures, fix memory leak

Summary

This code had a QHash and a separate ordered list of names in order
to use plugins in the right order. It's much simpler to just have a
vector of plugins instead. Also, this fixes a memory leak when multiple
plugins are found with the same name (e.g. one from /usr and one from
my custom prefix, although the right fix for this particular
issue is to load only one of them).

Test Plan

kurifilter tests still pass

Diff Detail

Repository

R241 KIO

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

dfaure created this revision.Dec 2 2017, 9:36 AM

Restricted Application added a project: Frameworks. · View Herald TranscriptDec 2 2017, 9:36 AM

dfaure requested review of this revision.Dec 2 2017, 9:36 AM

It's interesting how the code ends up simpler. :)

This revision is now accepted and ready to land.Dec 2 2017, 11:21 AM

dfaure planned changes to this revision.Dec 2 2017, 2:26 PM

dfaure added inline comments.

src/widgets/kurifilter.cpp
665 ↗	(On Diff #23254)	Interesting, to review my own commit from a month ago, after I forgot its details... Dude, this code crashes, reserve+transform needs back_inserter, otherwise use resize!

Closed by commit R241:73fd5f771cc4: KUriFilter: simplify data structures, fix memory leak (authored by dfaure). · Explain WhyDec 2 2017, 2:42 PM

This revision was automatically updated to reflect the committed changes.

emateli added a subscriber: emateli.Dec 2 2017, 2:49 PM

emateli added inline comments.

src/widgets/kurifilter.cpp
597 ↗	(On Diff #23254)	Perhaps this can be merged with the if statement above.

dfaure added inline comments.Dec 2 2017, 3:06 PM

src/widgets/kurifilter.cpp
597 ↗	(On Diff #23254)	It can, but I would certainly find it much less readable. The heart of the method is to call filterUri. The first if() is "should I call it?" The nested if() is "did it work?" A single if statement mixing both, would certainly make this harder to read.