D21146: KProcessInfoList -- add proclist backend for FreeBSD

[Adriaan de Groot]

adridg added inline comments.

INLINE COMMENTS

> adridg wrote in kprocesslist_unix_procstat.cpp:38
> I absolutely have Opinions on C++ style that apply here, so I'm going to talk to Tobias elsewhere.

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.

> kprocesslist_unix_procstat.cpp:46
> +    struct kinfo_proc *procs;
> +    procs = procstat_getprocs(pstat, KERN_PROC_PROC, 0, &proc_count);
> +

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.

REPOSITORY
  R244 KCoreAddons

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

To: tcberner, #freebsd, adridg, davidedmundson
Cc: pino, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190623/de2a74c0/attachment-0001.html>