Re: [PATCH v3] tr2: log parent process name

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

> diff --git a/compat/procinfo.c b/compat/procinfo.c
> new file mode 100644
> index 0000000000..0e92fb8b7c
> --- /dev/null
> +++ b/compat/procinfo.c
> @@ -0,0 +1,51 @@
> +#include "cache.h"
> +
> +#include "strbuf.h"
> +#include "trace2.h"
> +
> +char *get_process_name(int pid)
> +{
> +#ifdef HAVE_PROCFS_LINUX
> +	struct strbuf procfs_path = STRBUF_INIT;
> +	struct strbuf out = STRBUF_INIT;
> +	/* try to use procfs if it's present. */
> +	strbuf_addf(&procfs_path, "/proc/%d/comm", pid);
> +	if (!strbuf_read_file(&out, procfs_path.buf, 0)) {
> +		/* All done with file reads, clean up early */
> +		strbuf_release(&procfs_path);
> +		return strbuf_detach(&out, NULL);
> +	}
> +#endif
> +
> +	/* NEEDSWORK: add non-procfs implementations here. */
> +	return NULL;
> +}

Is the reason why this takes "int" and not "pid_t" because we may
port to non-POSIX platforms that do not have pid_t defined?

    ... goes and greps ...

Nah, we use pid_t everywhere (including compat/mingw.c); unless
there is a reason not to, let's use that type.

> +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) {
> +		/*
> +		 * NEEDSWORK: we could do the entire ptree in an array instead,
> +		 * see compat/win32/trace2_win32_process_info.c.
> +		 */
> +		char *names[2];
> +		names[0] = get_process_name(getppid());
> +		names[1] = NULL;

Makes me wonder if get_process_name() is an appropriate
abstraction; specifically, something like

		const char **names = get_ancestry_names();
                int cnt;
		if (names)
			trace2_cmd_ancestry(names);
		for (cnt = 0; names[cnt]; cnt++)
                	free((char *)names[cnt]);
		free(names);

would allow platforms to decide how many levels is easy for them to
grab for reporting, for example (and they do not even have to have
to assume that getting process IDs to feed get_process_name() one by
one is the easiest way to show ancestry).