Differential D21146 Diff 68151 src/lib/util/kprocesslist_unix_procstat.cpp

Changeset View

Standalone View

View Options

src/lib/util/kprocesslist_unix_procstat.cpp

  • This file was added.
1​/**************************************************************************
2​**
3​** This file is part of the KDE Frameworks
4​**
5​** Copyright (c) 2019 Tobias C. Berner <tcberner@FreeBSD.org>
6​**
7​** GNU Lesser General Public License Usage
8​**
9​** This file may be used under the terms of the GNU Lesser General Public
10​** License version 2.1 as published by the Free Software Foundation and
11​** appearing in the file LICENSE.LGPL included in the packaging of this file.
12​** Please review the following information to ensure the GNU Lesser General
13​** Public License version 2.1 requirements will be met:
14​** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
15​**
16​**************************************************************************/
17​
18​#include "kprocesslist.h"
19​#include "kprocesslist_unix_procstat_p.h"
20​
21​#include <QProcess>
22​#include <QDir>
23​
24​
25​using namespace KProcessList;
26​
27​// Determine UNIX processes by using the procstat library
28​KProcessInfoList KProcessList::processInfoList()
29​{
30​ KProcessInfoList rc;
31​
32​ ProcStat pstat;
33​ if (!pstat)
34​ {
35​ return rc;
36​ }
37​
38​ ProcStatProcesses procs(pstat);
39​ for (const auto& process_info: procs)
adridgUnsubmitted
Not Done

I absolutely have Opinions on C++ style that apply here, so I'm going to talk to Tobias elsewhere.

adridg: I absolutely have Opinions on C++ style that apply here, so I'm going to talk to Tobias…
adridgUnsubmitted
Not Done

In order to simplify cleanup, I'd use a RAII class as well:

/** @brief Wrapper around procstat_open_sysctl()
 * 
 * Hangs on to a procstat instance for its friend classes, and closes
 * it on destruction. Cast to bool to check for success.
 */
struct ProcStat
{
    ProcStat()
    {
        pstat = procstat_open_sysctl();
    }
    ~ProcStat()
    {
        procstat_close(pstat);
    }
    operator bool() const
    {
        return pstat;
    }
    
    struct procstat *pstat;
};

Lines 37-42 become ProcStat p; if (!p) return rc;, but more importantly you can drop cleanup from other return paths since the destructor handles it.

adridg: In order to simplify cleanup, I'd use a RAII class as well: ``` /** @brief Wrapper around…
40​ {
41​ rc.push_back(process_info);
42​ }
43​
44​ return rc;
45​}
adridgUnsubmitted
Not Done

Similarly, you could

struct ProcStatProcesses
{
public:
    ProcStatProcesses(ProcStat& pstat) : parent(pstat)
    {
        procs = procstat_getprocs(parent.pstat, KERN_PROC_PROC, 0, &proc_count);
    }
    ~ProcStatProcesses()
    {
        if (procs)
        {
            procstat_freeprocs(parent.pstat, procs);
        }
    }
    operator bool() const
    {
        return procs && proc_count > 0;
    }

    ProcStat& parent;
    unsigned int proc_count;
    struct kinfo_proc *procs;
}

I'd probably also write an iterator for it, returning KProcessInfo from operator*(), so that the magic is hidden away in small classes and the overall algorithm becomes a very short bit of text. You might call that over-engineered, since there's nothing wrong with the code as-written.

adridg: Similarly, you could ``` struct ProcStatProcesses { public: ProcStatProcesses(ProcStat&…