D21760: Add KListOpenFiles::listProcessesWithOpenFiles

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> klistopenfilestest.cpp:45
> +    eventLoop.exec();
> +    QVERIFY(processInfoList.empty() == false);
> +    auto testProcessIterator = std::find_if(processInfoList.begin(), processInfoList.end(), [](const KProcessList::KProcessInfo& info)

QVERIFY(!processInfoList.empty())

> klistopenfilestest.cpp:52
> +    const auto& processInfo = *testProcessIterator;
> +    QVERIFY(processInfo.isValid() == true);
> +    QCOMPARE(processInfo.pid(), QCoreApplication::applicationPid());

The `== true` can be removed

(repeats)

> klistopenfiles.cpp:39
> +        KProcessList::KProcessInfoList processInfoList;
> +        for (const auto& pidStr : pidList) {
> +            qint64 pid = pidStr.toLongLong();

qAsConst(pidList) to avoid a detach

> hallas wrote in klistopenfiles.cpp:49
> What should we do for Windows? Do we support any other platforms that doesn't have lsof?

This reminds me of KCoreAddons' new KProcessInfo class, although that one doesn't actually do this.

I see that FreeBSD and OSX have lsof (and the man page doesn't say those options are missing there) so it should be fine, everywhere except Windows.

In general I don't really like starting executables like this, but indeed writing a clone of lsof sounds like much work.

> hallas wrote in klistopenfiles.h:40
> I am very much in doubt about what a nice interface for this should be? I was thinking of an alternative approach where you would instantiate a class, connect to a signal on the class and then invoke a method to start the listing process, you would then receive the result on a slot instead. What do you guys think?

Reading the unittest, I definitely agree that a signal would be better. Then you could use QSignalSpy::wait() without the need to use QEventLoop by hand :-)

This is really a job. I would inherit KJob for this, actually. It's the way we do such things everywhere else.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D21760

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190724/601a4f94/attachment.html>