Re: [PATCH v2 3/6] trace2: refactor Windows process ancestry trace2 event
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 13, 2026, 20:36 UTC
- Message-ID
- <xmqqtsvkl6sm.fsf@gitster.g>
- In-Reply-To
- <2b02f62f0df7ad2ca5356813780577facf02a58d.1771012500.git.gitgitgadget@gmail.com>
"Matthew John Cheetham via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 17 quoted lines
> From: Matthew John Cheetham <mjcheetham@outlook.com> > > In 353d3d77f4 (trace2: collect Windows-specific process information, > 2019-02-22) we added process ancestry information for Windows to TRACE2 > via a data_json event. It was only later in 2f732bf15e (tr2: log parent > process name, 2021-07-21) that the specific cmd_ancestry event was > added to TRACE2. > > In a future commit we will emit the ancestry information with the newer > cmd_ancestry TRACE2 event. Right now, we rework this implementation of > trace2_collect_process_info to separate the calculation of ancestors > from building and emiting the JSON array via a data_json event. > > Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com> > --- > compat/win32/trace2_win32_process_info.c | 50 ++++++++++++------------ > 1 file changed, 25 insertions(+), 25 deletions(-)
Looks quite straight-forward refactoring that even I (without Win32 specific knowledge) can follow with ease. Nicely done.
Show 117 quoted lines
>
> diff --git a/compat/win32/trace2_win32_process_info.c b/compat/win32/trace2_win32_process_info.c
> index f147da706a..aceea05430 100644
> --- a/compat/win32/trace2_win32_process_info.c
> +++ b/compat/win32/trace2_win32_process_info.c
> @@ -3,6 +3,7 @@
> #include "../../git-compat-util.h"
> #include "../../json-writer.h"
> #include "../../repository.h"
> +#include "../../strvec.h"
> #include "../../trace2.h"
> #include "lazyload.h"
> #include <psapi.h>
> @@ -32,12 +33,7 @@ static int find_pid(DWORD pid, HANDLE hSnapshot, PROCESSENTRY32 *pe32)
> }
>
> /*
> - * Accumulate JSON array of our parent processes:
> - * [
> - * exe-name-parent,
> - * exe-name-grand-parent,
> - * ...
> - * ]
> + * Accumulate array of our parent process names.
> *
> * Note: we only report the filename of the process executable; the
> * only way to get its full pathname is to use OpenProcess()
> @@ -73,7 +69,7 @@ static int find_pid(DWORD pid, HANDLE hSnapshot, PROCESSENTRY32 *pe32)
> * simple and avoid the alloc/realloc overhead. It is OK if we
> * truncate the search and return a partial answer.
> */
> -static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
> +static void get_processes(struct strvec *names, HANDLE hSnapshot)
> {
> PROCESSENTRY32 pe32;
> DWORD pid;
> @@ -82,19 +78,19 @@ static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
>
> 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;
> }
>
> @@ -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);
> }
> }
> @@ -176,13 +162,27 @@ static void get_peak_memory_info(void)
>
> 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);
> + }
> +
> + strvec_clear(&names);
> return;
>
> case TRACE2_PROCESS_INFO_EXIT: