Re: [PATCH v6 2/2] tr2: log parent process name

[Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>]

On Wed, Jul 21 2021, Emily Shaffer wrote:

> Git for Windows also gathers information about more than one generation
> of parent. In Linux further ancestry info can be gathered with procfs,
> but it's unwieldy to do so.

Having read the win32 get_processes() implementation and read proc(5) I
don't get how it's unweildy to do so on Linux? Perhaps I'm missing some
special-case but this rather simple patch-on-top seems to do the job for
me. This includes the unrelated enum/switch/case change I suggested.

I can submit it as a patch-on-top with SOB etc, but maybe there's some
subtle reason it won't work properly. It works for me, I get e.g.:
    
    {
      "event": "cmd_ancestry",
      "sid": "20210802T102731.879424Z-Hc2f5b994-P00001acc",
      "thread": "main",
      "time": "2021-08-02T10:27:31.879618Z",
      "file": "compat/linux/procinfo.c",
      "line": 66,
      "ancestry": [
        "bash",
        "screen",
        "systemd"
      ]
    }

If anything the existing Windows version seems a bit unweildy, having to
apparently deal with cycles etc., I don't think that happens under
procfs, but maybe I'm missing some special-case.

diff --git a/compat/linux/procinfo.c b/compat/linux/procinfo.c
index 578fed4cd31..671fe2395fd 100644
--- a/compat/linux/procinfo.c
+++ b/compat/linux/procinfo.c
@@ -4,51 +4,65 @@
 #include "strvec.h"
 #include "trace2.h"
 
-static void get_ancestry_names(struct strvec *names)
+static int stat_parent_pid(FILE *fp, char *statcomm, int *statppid)
+{
+	char statstate;
+	int statpid;
+
+	int ret = fscanf(fp, "%d %s %c %d", &statpid, statcomm, &statstate,
+			 statppid);
+	if (ret != 4)
+		return -1;
+	return 0;
+}
+
+static void push_ancestry_name(struct strvec *names, pid_t pid)
 {
-	/*
-	 * NEEDSWORK: We could gather the entire pstree into an array to match
-	 * functionality with compat/win32/trace2_win32_process_info.c.
-	 * To do so, we may want to examine /proc/<pid>/stat. For now, just
-	 * gather the immediate parent name which is readily accessible from
-	 * /proc/$(getppid())/comm.
-	 */
 	struct strbuf procfs_path = STRBUF_INIT;
-	struct strbuf name = STRBUF_INIT;
+	char statcomm[PATH_MAX];
+	FILE *fp;
+	int ppid;
 
 	/* try to use procfs if it's present. */
-	strbuf_addf(&procfs_path, "/proc/%d/comm", getppid());
-	if (strbuf_read_file(&name, procfs_path.buf, 0)) {
-		strbuf_release(&procfs_path);
-		strbuf_trim_trailing_newline(&name);
-		strvec_push(names, strbuf_detach(&name, NULL));
-	}
+	strbuf_addf(&procfs_path, "/proc/%d/stat", pid);
+	fp = fopen(procfs_path.buf, "r");
+	if (!fp)
+		return;
 
-	return;
-	/* NEEDSWORK: add non-procfs-linux implementations here */
+	if (stat_parent_pid(fp, statcomm, &ppid) < 0)
+		return;
+
+	/*
+	 * The comm field is in parenthesis, use printf + offset as a
+	 * poor man's trimming of both ends.
+	 */
+	strvec_pushf(names, "%.*s", (int)strlen(statcomm) - 2, statcomm + 1);
+
+	/*
+	 * Both errors and reaching the end of the process chain are
+	 * reported as fields of 0 by proc(5)
+	 */
+	if (ppid != 0)
+		push_ancestry_name(names, ppid);
 }
 
 void trace2_collect_process_info(enum trace2_process_info_reason reason)
 {
-	if (!trace2_is_enabled())
-		return;
+	struct strvec names = STRVEC_INIT;
 
-	/* someday we may want to write something extra here, but not today */
-	if (reason == TRACE2_PROCESS_INFO_EXIT)
+	if (!trace2_is_enabled())
 		return;
 
-	if (reason == TRACE2_PROCESS_INFO_STARTUP) {
-		/*
-		 * NEEDSWORK: we could do the entire ptree in an array instead,
-		 * see compat/win32/trace2_win32_process_info.c.
-		 */
-		struct strvec names = STRVEC_INIT;
-
-		get_ancestry_names(&names);
+	switch (reason) {
+	case TRACE2_PROCESS_INFO_EXIT:
+		break;
+	case TRACE2_PROCESS_INFO_STARTUP:
+		push_ancestry_name(&names, getppid());
 
 		if (names.nr)
 			trace2_cmd_ancestry(names.v);
 		strvec_clear(&names);
+		break;
 	}
 
 	return;