Re: [PATCH v6 2/2] tr2: log parent process name
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-08-02 09:42:58
On Wed, Jul 21 2021, Emily Shaffer wrote:
+`"cmd_ancestry"`::
+ This event contains the text command name for the parent (and earlier
+ generations of parents) of the current process, in an array ordered from
+ nearest parent to furthest great-grandparent. It may not be implemented
+ on all platforms.
++
+------------
+{
+ "event":"cmd_ancestry",
+ ...
+ "ancestry":["bash","tmux: server","systemd"]
+}
+------------
+Okey, so because of later NEEDSWORK no system that runs systemd will currrently have output like this, just Windows.
+ /* + * 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. + */
This comment:
+ struct strbuf procfs_path = STRBUF_INIT;
+ struct strbuf name = STRBUF_INIT;
+
+ /* 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));
+ }
+
+ return;
+ /* NEEDSWORK: add non-procfs-linux implementations here */
+}
+
+void trace2_collect_process_info(enum trace2_process_info_reason reason)
+{
+ if (!trace2_is_enabled())
+ return;
+
+ /* someday we may want to write something extra here, but not today */
+ if (reason == TRACE2_PROCESS_INFO_EXIT)
+ return;
+
+ if (reason == TRACE2_PROCESS_INFO_STARTUP) {This should be a switch/case, so we get the compiler asserting/warning if we don't check enum arms, in this case there's just these two, let's have that be clear for readability.
+ /* + * NEEDSWORK: we could do the entire ptree in an array instead, + * see compat/win32/trace2_win32_process_info.c. + */
Seems to remove the need for this comment, i.e. we comment on this limitation of linux-specific get_ancestry_names twice, both in the function and its caller.
-
+ elsif ($event eq 'cmd_ancestry') {
+ # 'cmd_ancestry' is platform-specific and not implemented everywhere, so
+ # just skip it for testing purposes.
+ }The rest of this code uses two "\n" between elsif arms, let's be consistent here.