Re: [PATCH v6 2/2] tr2: log parent process name
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-08-02 10:30:34
Subsystem:
the rest · Maintainer:
Linus Torvalds
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;