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.
