From: Derrick Stolee <stolee@gmail.com>
Date: Mon, 09 Feb 2026 14:41:28 GMT
Subject: Re: [PATCH 3/4] trace2: refactor Windows process ancestry trace2 event
Message-ID: <1a282a1c-866e-49fc-b396-014921135cc4@gmail.com>
In-Reply-To: <7ccd0a9a6d89decaa5856494a184c71bc0d678e9.1770307510.git.gitgitgadget@gmail.com>

On 2/5/2026 11:05 AM, Matthew John Cheetham via GitGitGadget wrote:
> diff --git a/compat/win32/trace2_win32_process_info.c b/compat/win32/trace2_win32_process_info.c
...
>  #include "../../json-writer.h"

Are we able to delete this after your change?

>  	pid = GetCurrentProcessId();
>  	while (find_pid(pid, hSnapshot, &pe32)) {
> -		/* Only report parents. Omit self from the JSON output. */
> +		/* Only report parents. Omit self from the output. */
>  		if (nr_pids)
> -			jw_array_string(jw, pe32.szExeFile);
> +			strvec_push(names, pe32.szExeFile);
>  
>  		/* Check for cycle in snapshot. (Yes, it happened.) */
>  		for (k = 0; k < nr_pids; k++)
>  			if (pid == pid_list[k]) {
> -				jw_array_string(jw, "(cycle)");
> +				strvec_push(names, "(cycle)");
>  				return;
>  			}
>  
>  		if (nr_pids == NR_PIDS_LIMIT) {
> -			jw_array_string(jw, "(truncated)");
> +			strvec_push(names, "(truncated)");
>  			return;
>  		}

Nice replacement of JSON with strvec logic.

> @@ -105,24 +101,14 @@ static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
>  }
>  
>  /*
> - * Emit JSON data for the current and parent processes.  Individual
> - * trace2 targets can decide how to actually print it.
> + * Collect the list of parent process names.
>   */
> -static void get_ancestry(void)
> +static void get_ancestry(struct strvec *names)
>  {
>  	HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
>  
>  	if (hSnapshot != INVALID_HANDLE_VALUE) {
> -		struct json_writer jw = JSON_WRITER_INIT;
> -
> -		jw_array_begin(&jw, 0);
> -		get_processes(&jw, hSnapshot);
> -		jw_end(&jw);
> -
> -		trace2_data_json("process", the_repository, "windows/ancestry",
> -				 &jw);
> -
> -		jw_release(&jw);
> +		get_processes(names, hSnapshot);
>  		CloseHandle(hSnapshot);

Nice simplification!

>  void trace2_collect_process_info(enum trace2_process_info_reason reason)
>  {
> +	struct strvec names = STRVEC_INIT;
> +
>  	if (!trace2_is_enabled())
>  		return;
>  
>  	switch (reason) {
>  	case TRACE2_PROCESS_INFO_STARTUP:
>  		get_is_being_debugged();
> -		get_ancestry();
> +		get_ancestry(&names);
> +		if (names.nr) {
> +			struct json_writer jw = JSON_WRITER_INIT;
> +			jw_array_begin(&jw, 0);
> +			for (size_t i = 0; i < names.nr; i++)
> +				jw_array_string(&jw, names.v[i]);
> +			jw_end(&jw);
> +			trace2_data_json("process", the_repository,
> +					 "windows/ancestry", &jw);
> +			jw_release(&jw);

Ah, you still have JSON logic at this point.

I see that in your next patch you export the names vector itself _and_
this older JSON version. We should consider a future where we drop this
JSON altogether, but it's nice to have both for a few versions so tool
makers have time to respond.

Thanks,
-Stolee