bug#45729: [PATCH] system_process_attributes for OpenBSD

bug#45729: [PATCH] system_process_attributes for OpenBSD

[Omar Polo]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

Tags: patch

Hello,

The attached patch implements system_process_attributes for OpenBSD.
With this I am able to use proced \o/

There are five stats that I don't know how to collect (cminflt, cmajflt,
cstime, ctime and thcount).  Everything else seem consistent with
top(1).

I avoided to dynamically allocate the args string and instead reused the
existing storage; this is similar to the freebsd code, but if it seems
ugly I can replace that with a calloc+strlcat like in the previous patch
sent to emacs-devel@.

Note: I don't know how sysctl behaves on other systems, but at least on
OpenBSD it's not possible to get the argv for every process.  In my
environment only for a couple of process the KERN_PROC_ARGV sysctl
fails.  So, it's possible to see some entries with `?' as `args' in
proced, but you can verify that it's "expected" by running "ps $PID" and
see that you get "(executable-name)" under the COMMAND column.  (top(1)
seems to just hide them)

Should the patch include an entry in etc/NEWS?

Thanks,

Omar Polo

In GNU Emacs 28.0.50 (build 4, x86_64-unknown-openbsd6.8, X toolkit, cairo version 1.16.0, Xaw scroll bars)
 of 2021-01-05 built on venera
Repository revision: ce0c9c349363b5dbea56f7e32c838a93c729263d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: OpenBSD venera 6.8 GENERIC.MP#223 amd64

Configured using:
 'configure --prefix=/home/op/opt/emacs --with-x-toolkit=lucid'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: system_process_attribute-2.diff --]
[-- Type: text/patch, Size: 6035 bytes --]

diff 5ef30fbf11c3ee0ff1c30061dab554b9bd3ce841 /home/op/build/emacs
blob - 29c88f5308e47611e425f89a916eca578c86e09f
file + src/sysdep.c
--- src/sysdep.c
+++ src/sysdep.c
@@ -53,6 +53,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.or
 # include <sys/sysctl.h>
 #endif
 
+#if defined __OpenBSD__
+# include <sys/proc.h>
+#endif
+
 #ifdef DARWIN_OS
 # include <libproc.h>
 #endif
@@ -2984,6 +2988,14 @@ make_lisp_timeval (struct timeval t)
   return make_lisp_time (timeval_to_timespec (t));
 }
 
+#elif defined __OpenBSD__
+
+static Lisp_Object
+make_lisp_timeval (long sec, long usec)
+{
+  return make_lisp_time(make_timespec(sec, usec * 1000));
+}
+
 #endif
 
 #ifdef GNU_LINUX
@@ -3673,6 +3685,189 @@ system_process_attributes (Lisp_Object pid)
   return attrs;
 }
 
+#elif defined __OpenBSD__
+
+Lisp_Object
+system_process_attributes (Lisp_Object pid)
+{
+  int proc_id, nentries, fscale, i;
+  int pagesize = getpagesize ();
+  int mib[6];
+  size_t len;
+  double pct;
+  char *ttyname, args[ARG_MAX];
+  struct kinfo_proc proc;
+  struct passwd *pw;
+  struct group *gr;
+  struct timespec t;
+  struct uvmexp uvmexp;
+
+  Lisp_Object attrs = Qnil;
+  Lisp_Object decoded_comm;
+
+  CHECK_NUMBER (pid);
+  CONS_TO_INTEGER (pid, int, proc_id);
+
+  len = sizeof proc;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC;
+  mib[2] = KERN_PROC_PID;
+  mib[3] = proc_id;
+  mib[4] = len;
+  mib[5] = 1;
+  if (sysctl (mib, 6, &proc, &len, NULL, 0) != 0)
+    return attrs;
+
+  attrs = Fcons (Fcons (Qeuid, INT_TO_INTEGER (proc.p_uid)), attrs);
+
+  block_input ();
+  pw = getpwuid (proc.p_uid);
+  unblock_input ();
+  if (pw)
+    attrs = Fcons (Fcons (Quser, build_string(pw->pw_name)), attrs);
+
+  attrs = Fcons (Fcons (Qegid, INT_TO_INTEGER(proc.p_svgid)), attrs);
+
+  block_input ();
+  gr = getgrgid (proc.p_svgid);
+  unblock_input ();
+  if (gr)
+    attrs = Fcons (Fcons (Qgroup, build_string (gr->gr_name)), attrs);
+
+  AUTO_STRING (comm, proc.p_comm);
+  decoded_comm = code_convert_string_norecord (comm, Vlocale_coding_system, 0);
+  attrs = Fcons (Fcons (Qcomm, decoded_comm), attrs);
+
+  {
+    char state[2] = {'\0', '\0'};
+    switch (proc.p_stat) {
+    case SIDL:
+      state[0] = 'I';
+      break;
+    case SRUN:
+      state[0] = 'R';
+      break;
+    case SSLEEP:
+      state[0] = 'S';
+      break;
+    case SSTOP:
+      state[0] = 'T';
+      break;
+    case SZOMB:
+      state[0] = 'Z';
+      break;
+    case SDEAD:
+      state[0] = 'D';
+      break;
+    }
+    attrs = Fcons (Fcons (Qstate, build_string (state)), attrs);
+  }
+
+  attrs = Fcons (Fcons (Qppid, INT_TO_INTEGER (proc.p_ppid)), attrs);
+  attrs = Fcons (Fcons (Qpgrp, INT_TO_INTEGER (proc.p_gid)), attrs);
+  attrs = Fcons (Fcons (Qsess, INT_TO_INTEGER (proc.p_sid)),  attrs);
+
+  block_input ();
+  ttyname = proc.p_tdev == NODEV ? NULL : devname (proc.p_tdev, S_IFCHR);
+  unblock_input ();
+  if (ttyname)
+    attrs = Fcons (Fcons (Qttname, build_string (ttyname)), attrs);
+
+  attrs = Fcons (Fcons (Qtpgid,   INT_TO_INTEGER (proc.p_tpgid)), attrs);
+  attrs = Fcons (Fcons (Qminflt,  INT_TO_INTEGER (proc.p_uru_minflt)),
+		 attrs);
+  attrs = Fcons (Fcons (Qmajflt,  INT_TO_INTEGER (proc.p_uru_majflt)),
+		 attrs);
+
+  /* FIXME: missing cminflt, cmajflt. */
+
+  attrs = Fcons (Fcons (Qutime, make_lisp_timeval (proc.p_uutime_sec,
+						   proc.p_uutime_usec)),
+		 attrs);
+  attrs = Fcons (Fcons (Qstime, make_lisp_timeval (proc.p_ustime_sec,
+						   proc.p_ustime_usec)),
+		 attrs);
+  t = timespec_add (make_timespec (proc.p_uutime_sec,
+				   proc.p_uutime_usec * 1000),
+		    make_timespec (proc.p_ustime_sec,
+				   proc.p_ustime_usec * 1000));
+  attrs = Fcons (Fcons (Qtime, make_lisp_time (t)), attrs);
+
+  attrs = Fcons (Fcons (Qcutime, make_lisp_timeval (proc.p_uctime_sec,
+						    proc.p_uctime_usec)),
+		 attrs);
+
+  /* FIXME: missing cstime and thus ctime. */
+
+  attrs = Fcons (Fcons (Qpri,   make_fixnum (proc.p_priority)), attrs);
+  attrs = Fcons (Fcons (Qnice,  make_fixnum (proc.p_nice)), attrs);
+
+  /* FIXME: missing thcount (thread count) */
+
+  attrs = Fcons (Fcons (Qstart, make_lisp_timeval (proc.p_ustart_sec,
+						   proc.p_ustart_usec)),
+		 attrs);
+
+  len = (proc.p_vm_tsize + proc.p_vm_dsize + proc.p_vm_ssize) * pagesize >> 10;
+  attrs = Fcons (Fcons (Qvsize, make_fixnum (len)), attrs);
+
+  attrs = Fcons (Fcons (Qrss,   make_fixnum (proc.p_vm_rssize * pagesize >> 10)),
+		 attrs);
+
+  t = make_timespec (proc.p_ustart_sec,
+		     proc.p_ustart_usec * 1000);
+  t = timespec_sub (current_timespec (), t);
+  attrs = Fcons (Fcons (Qetime, make_lisp_time (t)), attrs);
+
+  len = sizeof (fscale);
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_FSCALE;
+  if (sysctl (mib, 2, &fscale, &len, NULL, 0) != -1)
+    {
+      pct = (double)proc.p_pctcpu / fscale * 100.0;
+      attrs = Fcons (Fcons (Qpcpu, make_float (pct)), attrs);
+    }
+
+  len = sizeof (uvmexp);
+  mib[0] = CTL_VM;
+  mib[1] = VM_UVMEXP;
+  if (sysctl (mib, 2, &uvmexp, &len, NULL, 0) != -1)
+    {
+      pct = (100.0 * (double)proc.p_vm_rssize / uvmexp.npages);
+      attrs = Fcons (Fcons (Qpmem, make_float (pct)), attrs);
+    }
+
+  len = sizeof args;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC_ARGS;
+  mib[2] = proc_id;
+  mib[3] = KERN_PROC_ARGV;
+  if (sysctl (mib, 4, &args, &len, NULL, 0) == 0 && len != 0)
+    {
+      char **argv = (char**)args;
+
+      /* concatenate argv reusing the existing storage storage.
+	 sysctl(8) guarantees that "the buffer pointed to by oldp is
+	 filled with an array of char pointers followed by the strings
+	 themselves." */
+      for (i = 0; argv[i] != NULL; ++i)
+	{
+	  if (argv[i+1] != NULL)
+	    {
+	      len = strlen (argv[i]);
+	      argv[i][len] = ' ';
+	    }
+	}
+
+      AUTO_STRING (comm, *argv);
+      decoded_comm = code_convert_string_norecord (comm,
+						   Vlocale_coding_system, 0);
+      attrs = Fcons (Fcons (Qargs, decoded_comm), attrs);
+    }
+
+  return attrs;
+}
+
 #elif defined DARWIN_OS
 
 Lisp_Object

bug#45729: [PATCH] system_process_attributes for OpenBSD

[Lars Ingebrigtsen]

Omar Polo <op@omarpolo.com> writes:

> The attached patch implements system_process_attributes for OpenBSD.
> With this I am able to use proced \o/

Looks good to me, and I've tested it slightly on OpenBSD, and it seems
to work fine, so I've now pushed it to Emacs 28.  (I see that your
copyright is on file...  er...  OK, your entry has a timestamp of
"2021-07-07", so that makes it even better -- it's from the future!)

(Oh, one minor note for future patches -- using "git format-patch" (or
something similar) to format it is usually a good idea, because then
it'll apply automatically.  I had to do a "patch -p0" here.)

> Should the patch include an entry in etc/NEWS?

I've added one.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45729: [PATCH] system_process_attributes for OpenBSD

[Omar Polo]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> The attached patch implements system_process_attributes for OpenBSD.
>> With this I am able to use proced \o/
>
> Looks good to me, and I've tested it slightly on OpenBSD, and it seems
> to work fine, so I've now pushed it to Emacs 28.  (I see that your
> copyright is on file...  er...  OK, your entry has a timestamp of
> "2021-07-07", so that makes it even better -- it's from the future!)

Sorry, I don't understand this.  Do you mean the date in my copyright
assignment?  On the pdf I have, both my sign and the fsf one are dated
2021-01-07.  I'm missing something?

> (Oh, one minor note for future patches -- using "git format-patch" (or
> something similar) to format it is usually a good idea, because then
> it'll apply automatically.  I had to do a "patch -p0" here.)

Sorry, I was playing with another vcs and I forgot to re-test the patch
before sending.  I'll pay more attention in the future.

>> Should the patch include an entry in etc/NEWS?
>
> I've added one.

Thanks!

Omar Polo

bug#45729: [PATCH] system_process_attributes for OpenBSD

[Lars Ingebrigtsen]

Omar Polo <op@omarpolo.com> writes:

>> Looks good to me, and I've tested it slightly on OpenBSD, and it seems
>> to work fine, so I've now pushed it to Emacs 28.  (I see that your
>> copyright is on file...  er...  OK, your entry has a timestamp of
>> "2021-07-07", so that makes it even better -- it's from the future!)
>
> Sorry, I don't understand this.  Do you mean the date in my copyright
> assignment?  On the pdf I have, both my sign and the fsf one are dated
> 2021-01-07.  I'm missing something?

Sorry, just being silly.  It looks like all the entries in the database
on the FSF side from that date is marked as 2021-07-07 instead of
2021-01-07 (just a typo from somebody).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45729: [PATCH] system_process_attributes for OpenBSD

[Omar Polo]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>>> Looks good to me, and I've tested it slightly on OpenBSD, and it seems
>>> to work fine, so I've now pushed it to Emacs 28.  (I see that your
>>> copyright is on file...  er...  OK, your entry has a timestamp of
>>> "2021-07-07", so that makes it even better -- it's from the future!)
>>
>> Sorry, I don't understand this.  Do you mean the date in my copyright
>> assignment?  On the pdf I have, both my sign and the fsf one are dated
>> 2021-01-07.  I'm missing something?
>
> Sorry, just being silly.  It looks like all the entries in the database
> on the FSF side from that date is marked as 2021-07-07 instead of
> 2021-01-07 (just a typo from somebody).

I was worried I've done something wrong, sorry.  (overly so)

Thanks again,

Omar Polo