bug#48548: 28.0.50; Some process attributes on macOS are missing

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On macOS, system_process_attributes() could provide more attributes
("args", in particular).  Will post patch here.

Thanks

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

tags 48548 + patch
quit

On 20/05/2021 23:24 +0300, Filipp Gunbin wrote:

> On macOS, system_process_attributes() could provide more attributes
> ("args", in particular).  Will post patch here.
>
> Thanks

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

  Improve system_process_attributes on macOS (Bug#48548)
  
  * src/sysdep.c (system_process_attributes): Fix misprint in 'tty' attr
  - should be 'ttname' instead.  Change 'utime', 'stime', 'time',
  'majflt' attrs to obtain them from proc_pid_rusage, as sysctl call
  used before doesn't give correct values; remove 'minflt' because it's
  not available.  Obtain 'vsize' / 'rss' / 'thcount' from proc_pidinfo.
  Use sysctl with KERN_PROCARGS2 to obtain args: value contains both
  argc and argv, so argv can be reliably cut out.


diff --git a/src/sysdep.c b/src/sysdep.c
index d940acc4e0..f899bb7532 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -3898,20 +3898,19 @@ system_process_attributes (Lisp_Object pid)
 Lisp_Object
 system_process_attributes (Lisp_Object pid)
 {
-  int proc_id;
+  int proc_id, i;
   struct passwd *pw;
   struct group  *gr;
   char *ttyname;
   struct timeval starttime;
   struct timespec t, now;
-  struct rusage *rusage;
   dev_t tdev;
   uid_t uid;
   gid_t gid;
 
   int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID};
   struct kinfo_proc proc;
-  size_t proclen = sizeof proc;
+  size_t len = sizeof proc;
 
   Lisp_Object attrs = Qnil;
   Lisp_Object decoded_comm;
@@ -3920,7 +3919,7 @@ system_process_attributes (Lisp_Object pid)
   CONS_TO_INTEGER (pid, int, proc_id);
   mib[3] = proc_id;
 
-  if (sysctl (mib, 4, &proc, &proclen, NULL, 0) != 0 || proclen == 0)
+  if (sysctl (mib, 4, &proc, &len, NULL, 0) != 0 || len == 0)
     return attrs;
 
   uid = proc.kp_eproc.e_ucred.cr_uid;
@@ -3957,8 +3956,8 @@ system_process_attributes (Lisp_Object pid)
   decoded_comm = (code_convert_string_norecord
 		  (build_unibyte_string (comm),
 		   Vlocale_coding_system, 0));
-
   attrs = Fcons (Fcons (Qcomm, decoded_comm), attrs);
+
   {
     char state[2] = {'\0', '\0'};
     switch (proc.kp_proc.p_stat)
@@ -3994,27 +3993,24 @@ system_process_attributes (Lisp_Object pid)
   ttyname = tdev == NODEV ? NULL : devname (tdev, S_IFCHR);
   unblock_input ();
   if (ttyname)
-    attrs = Fcons (Fcons (Qtty, build_string (ttyname)), attrs);
+    attrs = Fcons (Fcons (Qttname, build_string (ttyname)), attrs);
 
   attrs = Fcons (Fcons (Qtpgid, INT_TO_INTEGER (proc.kp_eproc.e_tpgid)),
 		 attrs);
 
-  rusage = proc.kp_proc.p_ru;
-  if (rusage)
+  rusage_info_current ri;
+  if (proc_pid_rusage(proc_id, RUSAGE_INFO_CURRENT, (rusage_info_t *) &ri) == 0)
     {
-      attrs = Fcons (Fcons (Qminflt, INT_TO_INTEGER (rusage->ru_minflt)),
-		     attrs);
-      attrs = Fcons (Fcons (Qmajflt, INT_TO_INTEGER (rusage->ru_majflt)),
-		     attrs);
-
-      attrs = Fcons (Fcons (Qutime, make_lisp_timeval (rusage->ru_utime)),
-		     attrs);
-      attrs = Fcons (Fcons (Qstime, make_lisp_timeval (rusage->ru_stime)),
-		     attrs);
-      t = timespec_add (timeval_to_timespec (rusage->ru_utime),
-			timeval_to_timespec (rusage->ru_stime));
-      attrs = Fcons (Fcons (Qtime, make_lisp_time (t)), attrs);
-    }
+      struct timespec utime = make_timespec (ri.ri_user_time / TIMESPEC_HZ,
+					     ri.ri_user_time % TIMESPEC_HZ);
+      struct timespec stime = make_timespec (ri.ri_system_time / TIMESPEC_HZ,
+					     ri.ri_system_time % TIMESPEC_HZ);
+      attrs = Fcons (Fcons (Qutime, make_lisp_time (utime)), attrs);
+      attrs = Fcons (Fcons (Qstime, make_lisp_time (stime)), attrs);
+      attrs = Fcons (Fcons (Qtime, make_lisp_time (timespec_add (utime, stime))), attrs);
+
+      attrs = Fcons (Fcons (Qmajflt, INT_TO_INTEGER (ri.ri_pageins)), attrs);
+  }
 
   starttime = proc.kp_proc.p_starttime;
   attrs = Fcons (Fcons (Qnice,  make_fixnum (proc.kp_proc.p_nice)), attrs);
@@ -4024,6 +4020,50 @@ system_process_attributes (Lisp_Object pid)
   t = timespec_sub (now, timeval_to_timespec (starttime));
   attrs = Fcons (Fcons (Qetime, make_lisp_time (t)), attrs);
 
+  struct proc_taskinfo taskinfo;
+  if (proc_pidinfo (proc_id, PROC_PIDTASKINFO, 0, &taskinfo, sizeof (taskinfo)) > 0)
+    {
+      attrs = Fcons (Fcons (Qvsize, make_fixnum (taskinfo.pti_virtual_size / 1024)), attrs);
+      attrs = Fcons (Fcons (Qrss, make_fixnum (taskinfo.pti_resident_size / 1024)), attrs);
+      attrs = Fcons (Fcons (Qthcount, make_fixnum (taskinfo.pti_threadnum)), attrs);
+    }
+
+#ifdef KERN_PROCARGS2
+  char args[ARG_MAX];
+  mib[1] = KERN_PROCARGS2;
+  mib[2] = proc_id;
+  len = sizeof args;
+
+  if (sysctl (mib, 3, &args, &len, NULL, 0) == 0 && len != 0)
+    {
+      char *start, *end;
+
+      int argc = *(int*)args; /* argc is the first int */
+      start = args + sizeof (int);
+
+      start += strlen (start) + 1; /* skip executable name and any '\0's */
+      while ((start - args < len) && ! *start) start++;
+
+      /* skip argv to find real end */
+      for (i = 0, end = start; i < argc && (end - args) < len; i++)
+	{
+	  end += strlen (end) + 1;
+	}
+
+      len = end - start;
+      for (int i = 0; i < len; i++)
+	{
+	  if (! start[i] && i < len - 1)
+	    start[i] = ' ';
+	}
+
+      AUTO_STRING (comm, start);
+      decoded_comm = code_convert_string_norecord (comm,
+						   Vlocale_coding_system, 0);
+      attrs = Fcons (Fcons (Qargs, decoded_comm), attrs);
+    }
+#endif	/* KERN_PROCARGS2 */
+
   return attrs;
 }

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

Hello, could someone please review this? (bug#48548)

I'm kinda sure in syscalls I make (tested them on latest Big Sur 11.3.1
and on Yosemite from several years ago; running with this patch for
several weeks now, using via M-x proced), but I'm not so sure in my C
:-)

Thanks.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Thu, 20 May 2021 23:49:54 +0300
> Cc: bug-gnu-emacs@gnu.org
> 
> Hello, could someone please review this? (bug#48548)

Please be more patient: you posted the patch just 10 minutes before
sending this ping.  The patch will be reviewed soon enough, just give
us some time.

Thanks.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On 21/05/2021 08:53 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Thu, 20 May 2021 23:49:54 +0300
>> Cc: bug-gnu-emacs@gnu.org
>>
>> Hello, could someone please review this? (bug#48548)
>
> Please be more patient: you posted the patch just 10 minutes before
> sending this ping.  The patch will be reviewed soon enough, just give
> us some time.
>
> Thanks.

Pinging wasn't my intent, I just wanted to ask for review on
emacs-devel..

Thanks.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Fri, 21 May 2021 12:50:50 +0300
> Cc: bug-gnu-emacs@gnu.org, emacs-devel@gnu.org
> 
> Pinging wasn't my intent, I just wanted to ask for review on
> emacs-devel..

Cross-posting on both bug-gnu-emacs and emacs-devel is generally not a
good idea, so please try not to do that in the future.  The people who
are likely to review your patch are reading the bug list anyway.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On 21/05/2021 13:27 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Fri, 21 May 2021 12:50:50 +0300
>> Cc: bug-gnu-emacs@gnu.org, emacs-devel@gnu.org
>>
>> Pinging wasn't my intent, I just wanted to ask for review on
>> emacs-devel..
>
> Cross-posting on both bug-gnu-emacs and emacs-devel is generally not a
> good idea, so please try not to do that in the future.  The people who
> are likely to review your patch are reading the bug list anyway.

Yes, I know that crossposting is not generally appreciated, but in this
case it's a bit different: I'm not posting the same message, rather I'm
creating a bug as a container for patch (I wanted to try debbugs
package, although I failed to do it properly, and the tagging message
went without patch - now I see where I was wrong, I didn't select the
commit _range_ with debbugs-gnu-pick-commits, but rather just a commit),
and the review request as an actual message.

What is best to do such things in the future?  Just post a bug with tag
"patch", or just directly post patch to emacs-devel?  I don't want to
cause inconvenience, sorry for this time.

Filipp

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: emacs-devel@gnu.org
> Date: Fri, 21 May 2021 15:30:38 +0300
> 
> Yes, I know that crossposting is not generally appreciated, but in this
> case it's a bit different: I'm not posting the same message, rather I'm
> creating a bug as a container for patch (I wanted to try debbugs
> package, although I failed to do it properly, and the tagging message
> went without patch - now I see where I was wrong, I didn't select the
> commit _range_ with debbugs-gnu-pick-commits, but rather just a commit),
> and the review request as an actual message.
> 
> What is best to do such things in the future?  Just post a bug with tag
> "patch", or just directly post patch to emacs-devel?  I don't want to
> cause inconvenience, sorry for this time.

Just post to the bug list and wait for responses.  If you see no
responses in a week or two, ping the bug list.  If that doesn't
receive any responses, either, ask on emacs-devel about the bug.

It is rare to have a patch unanswered for a prolonged time, unless
it's in some very obscure part of Emacs.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On 21/05/2021 15:33 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 21 May 2021 15:30:38 +0300
>>
>> Yes, I know that crossposting is not generally appreciated, but in this
>> case it's a bit different: I'm not posting the same message, rather I'm
>> creating a bug as a container for patch (I wanted to try debbugs
>> package, although I failed to do it properly, and the tagging message
>> went without patch - now I see where I was wrong, I didn't select the
>> commit _range_ with debbugs-gnu-pick-commits, but rather just a commit),
>> and the review request as an actual message.
>>
>> What is best to do such things in the future?  Just post a bug with tag
>> "patch", or just directly post patch to emacs-devel?  I don't want to
>> cause inconvenience, sorry for this time.
>
> Just post to the bug list and wait for responses.  If you see no
> responses in a week or two, ping the bug list.  If that doesn't
> receive any responses, either, ask on emacs-devel about the bug.

You mean normal M-x report-emacs-bug, or just posting to the list?

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Fri, 21 May 2021 15:36:34 +0300
> Cc: emacs-devel@gnu.org
> 
> > Just post to the bug list and wait for responses.  If you see no
> > responses in a week or two, ping the bug list.  If that doesn't
> > receive any responses, either, ask on emacs-devel about the bug.
> 
> You mean normal M-x report-emacs-bug, or just posting to the list?

It doesn't matter much, when you are posting a patch.  Up to you.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Eli Zaretskii]

> Date: Fri, 21 May 2021 15:45:12 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > You mean normal M-x report-emacs-bug, or just posting to the list?
> 
> It doesn't matter much, when you are posting a patch.  Up to you.

Of course, if the patch is accompanied by a report of a bug, then
report-emacs-bug is better, since it collects data that could be
relevant for analyzing the bug.

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On 21/05/2021 15:45 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Fri, 21 May 2021 15:36:34 +0300
>> Cc: emacs-devel@gnu.org
>>
>> > Just post to the bug list and wait for responses.  If you see no
>> > responses in a week or two, ping the bug list.  If that doesn't
>> > receive any responses, either, ask on emacs-devel about the bug.
>>
>> You mean normal M-x report-emacs-bug, or just posting to the list?
>
> It doesn't matter much, when you are posting a patch.  Up to you.

Thanks!

Re: bug#48548: 28.0.50; Some process attributes on macOS are missing

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 21 May 2021 15:45:12 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > You mean normal M-x report-emacs-bug, or just posting to the list?
>> 
>> It doesn't matter much, when you are posting a patch.  Up to you.
>
> Of course, if the patch is accompanied by a report of a bug, then
> report-emacs-bug is better, since it collects data that could be
> relevant for analyzing the bug.

FWIW, Emacs 28 adds yet another alternative:

*** New command 'submit-emacs-patch'.
This works like 'report-emacs-bug', but is more geared towards sending
patches to the Emacs issue tracker.

-- 
Basil

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Lars Ingebrigtsen]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

>   Improve system_process_attributes on macOS (Bug#48548)

Looks good to me, but I've added Alan to the CCs -- perhaps he'll have
some comments.

Just one small comment:

> -  size_t proclen = sizeof proc;
> +  size_t len = sizeof proc;

Any reason for this change?

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

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

On 25/05/2021 06:08 +0200, Lars Ingebrigtsen wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
>>   Improve system_process_attributes on macOS (Bug#48548)
>
> Looks good to me, but I've added Alan to the CCs -- perhaps he'll have
> some comments.

Thanks, will wait.

> Just one small comment:
>
>> -  size_t proclen = sizeof proc;
>> +  size_t len = sizeof proc;
>
> Any reason for this change?

Yes, the usage of len for different purpose below:

+  len = sizeof args;

Filipp

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Lars Ingebrigtsen]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

>> Any reason for this change?
>
> Yes, the usage of len for different purpose below:
>
> +  len = sizeof args;

Oops; didn't read the patch carefully enough.

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

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Alan Third]

On Tue, May 25, 2021 at 06:08:56AM +0200, Lars Ingebrigtsen wrote:
> Filipp Gunbin <fgunbin@fastmail.fm> writes:
> 
> >   Improve system_process_attributes on macOS (Bug#48548)
> 
> Looks good to me, but I've added Alan to the CCs -- perhaps he'll have
> some comments.

No comments from me.
-- 
Alan Third

bug#48548: 28.0.50; Some process attributes on macOS are missing

[Filipp Gunbin]

tags 48548 fixed
close 48548 28.1
quit

On 26/05/2021 07:52 +0100, Alan Third wrote:

> On Tue, May 25, 2021 at 06:08:56AM +0200, Lars Ingebrigtsen wrote:
>> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>> 
>> >   Improve system_process_attributes on macOS (Bug#48548)
>> 
>> Looks good to me, but I've added Alan to the CCs -- perhaps he'll have
>> some comments.
>
> No comments from me.
> -- 
> Alan Third

Pushed to master.

6d51805154 2021-05-26T16:15:03+03:00 "Improve system_process_attributes on macOS (Bug#48548)"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6d51805154ef7591917c5727b905b4080e18b888