[Konsole-devel] Review Request: Add process info gathering support on OpenBSD

[Vadim Zhukov]

03.10.2012 13:57 пользователь "Jekyll Wu" <adaptee at gmail.com> написал:
>
> This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106129/
>
> Sorry for the so late response. I have been blocked by setting up an
OpenBSD environment for testing this patch. The funny thing is in the same
time I have managed to set up the testing environment for NetBSD and
DragonFly without much trouble :)

M-m-m... Official OpenBSD Ports tree doesn't have (working) KDE 4; I'm
actually working on it. This patch was tested at least by one another
person. I can help you in setting up OpenBSD testing environment, if you
wish.

> Back to this patch. Since I (and probably Kurt also) can't test it now,
the safe way is to prepare a patch which does not contain any modification
to existing code/class.  So please use a separate class for OpenBSD.

Acknowledged. I'll send new revision ASAP.

> Also, leave alone that fix for allocating kinfo_proc in the wrong unit in
class FreeBSDProcessInfo. I will try to fix it in a separate commit.

Okay.

> Beside that, there are some minor coding style issues.

Will fix them too. Sorry for C-style, it's much easier to follow one manual
(OpenBSD) than for (OpenBSD, Qt and KDE). :)

> src/ProcessInfo.cpp (Diff revision 2)
>
> private:
>
> 681
>
>         void    *buf = NULL, *nbuf;
>
> konsole code generally uses the form of
>
>    void* ptr = ...;
>
> Also, it is preferred to have only one declaration per line.
>
> There are similar issues in the following code.
>
>
> src/ProcessInfo.cpp (Diff revision 2)
>
> private:
>
> 719
>
>         for (char **p = argv; *p != NULL; p++)
>
> konsole code generally uses the kdelibs style:
>
> for (....) {
>
> }
>
>
> src/ProcessInfo.cpp (Diff revision 2)
>
> private:
>
> 754
>
>         envp = readProcArgs(aPid, KERN_PROC_ENV);
>
> This part feels so "C", which makes it out of place among other "C++"
code. Also applies to the readProcArgs method above.
>
> But that is just my taste. If you feel uncertain about making it more
"C++" like, just ignore this issue.
>
>
> - Jekyll
>
>
> On August 23rd, 2012, 7:29 p.m., Vadim Zhukov wrote:
>
> Review request for Konsole and Jekyll Wu.
> By Vadim Zhukov.
>
> Updated Aug. 23, 2012, 7:29 p.m.
>
> Description
>
> ATM Konsole does not support gathering process information on OpenBSD.
I've a patch adding such support.
> The main idea is to extend already existing FreeBSDProcessInfo class. I
renamed it to a BSDProcessInfo and added a few #ifs here and there. If
that's not desirable, I could create separate OpenBSDProcessInfo class.
>
> I've also fixed a few nits regarding calling C routines (prepended "::"
to sysctl() and such; added kWarning() printouts if they fail). If needed I
can do them as a separate diff.
>
> Testing
>
> On OpenBSD-CURRENT.
>
> Diffs
> src/ProcessInfo.cpp (32c86b1)
>
> View Diff
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20121003/afdcd04c/attachment.html>