From: Adrian Ratiu Date: Thu, 27 Nov 2025 12:00:18 GMT Subject: Re: [PATCH v3 01/10] run-command: add stdin callback for parallelization Message-ID: <87tsyfit2l.fsf@gentoo.mail-host-address-is-not-set> In-Reply-To: On Tue, 25 Nov 2025, Junio C Hamano wrote: > Adrian Ratiu writes: > >> +static void pp_buffer_stdin(struct parallel_processes *pp, + >> const struct run_process_parallel_opts *opts) +{ + /* Buffer >> stdin for each pipe. */ + for (size_t i = 0; i < >> opts->processes; i++) { + struct child_process *proc >> = &pp->children[i].process; + int ret; + + >> if (pp->children[i].state != GIT_CP_WORKING || proc->in <= 0) + >> continue; > > This combination of two conditions is a recurring theme in this > series (e.g., something close to the reverse of this is called > child-ready-for-cleanup later). > > I wonder if it makes the code easier to follow if we add a set > of helpers that take a pointer to &pp->children[i] (and we > probably should give the struct that describes each child > managed in the "struct parallel_processes" some name) and asks > "is this thing done?" etc. I think this is a good idea and worth exploring. Will see what I can come up and if this makes code patterns clearer. Thanks! >> diff --git a/run-command.h b/run-command.h index >> 0df25e445f..e536ed7544 100644 --- a/run-command.h +++ >> b/run-command.h @@ -420,6 +420,22 @@ typedef int >> (*start_failure_fn)(struct strbuf *out, >> void *pp_cb, void *pp_task_cb); >> +/** + * This callback is repeatedly called on every child >> process who requests + * start_command() to create a pipe by >> setting child_process.in < 0. + * + * pp_cb is the callback >> cookie as passed into run_processes_parallel, and + * >> pp_task_cb is the callback cookie as passed into >> get_next_task_fn. + * The contents of 'send' will be read into >> the pipe and passed to the pipe. > > There is no 'send' seen around here. Does this refer to > something a later step adds? Good catch. This is a leftover comment from one of the previous patch iterations. Will remove.