As per apol's suggestion in D26210, use splitRef(). And Since it returns
a QVector<String>, use std::unique to remove the duplicates.
Details
- Reviewers
mpyne dfaure apol - Group Reviewers
Frameworks - Maniphest Tasks
- T12279: Port frameworks away from QRegExp
- Commits
- R244:4bcf55e06c2f: [KListOpenFilesJob] Use QString::splitRef()
make && ctest
Diff Detail
- Repository
- R244 KCoreAddons
- Branch
- l-splitref (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20286 Build 20304: arc lint + arc unit
src/lib/util/klistopenfilesjob_unix.cpp | ||
---|---|---|
71 ↗ | (On Diff #72188) | This does not work the way you think it does. "A call to std::unique is typically followed by a call to a container's erase method, which erases the unspecified values and reduces the physical size of the container to match its new logical size." Also, note that unlike removeDuplicates(), std::unique requires the container to be sorted (or at least the duplicates to be consecutive). But AFAICS the output of lsof is already sorted? In fact in all my tests I don't manage to see it output duplicates... Are you sure this can happen? |
src/lib/util/klistopenfilesjob_unix.cpp | ||
---|---|---|
71 ↗ | (On Diff #72188) | Ouch, right; I should have read up fully on unique before using it (I skimmed the docs, always a stupid idea with a new tool). removeDuplicates() was in the original code; and lsof would have duplicate pids, but I missed that the code actually uses lsof -t which is both sorted and unique. I'll investigate some more and update the diff. |
Remove the call to std::unique:
- It doesn't work like that
- it's not needed as lsof -t already produces a sorted and unique list of pid's
One more thing, since this is lsof -t it's only 52 entries on my system (not +100000 of lsof), so it's not as big of an optimisation as I first thought (see D26226 for context).
src/lib/util/klistopenfilesjob_unix.cpp | ||
---|---|---|
69 | Now you can make it const and remove the qAsConst on the next line. |