As per apol's suggestion in D26210, use splitRef(). And Since it returns
a QVector<String>, use std::unique to remove the duplicates.
Details
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
Diff Detail
- Repository
- R244 KCoreAddons
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/lib/util/klistopenfilesjob_unix.cpp | ||
|---|---|---|
| 70 | 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 | ||
|---|---|---|
| 70 | 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. | |
Comment Actions
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
Comment Actions
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 | ||
|---|---|---|
| 68–69 | Now you can make it const and remove the qAsConst on the next line. | |