Re: [PATCH 2/5] run-command: Call get_next_task with a clean child process.

> Stefan Beller <sbeller@google.com> writes:
>
>> If the `get_next_task` did not explicitly called child_process_init
>
> I locally did "If get_next_task did not explicitly call child_process_init"
>
>> and only filled in some fields, there may have been some stale data
>> in the child process. This is hard to debug and also adds a review
>> burden for each new user of that API. To improve the situation, we
>> pass only cleanly initialized child structs to the get_next_task.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> Again this makes sense.
>
> Another way, which I suspect may be conceptually cleaner, would be
> to do this clean-up where pp->children[i].in_use is set to false, as
> that is where the particular task is declared to be available for
> reuse.  That location should be responsible to ensure that the task
> indeed is reusable by calling child_process_init().
>
> By the way, does child_process_init() leak old argv/env arrays, or
> have they already been cleared by calling finish_command() when we
> come to this codepath?

>
>>  run-command.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/run-command.c b/run-command.c
>> index 8f47c6e..b8b5747 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -1010,6 +1010,8 @@ static int pp_start_one(struct parallel_processes *pp)
>>       if (i == pp->max_processes)
>>               die("BUG: bookkeeping is hard");
>>
>> +     child_process_init(&pp->children[i].process);
>> +
>>       if (!pp->get_next_task(&pp->children[i].data,
>>                              &pp->children[i].process,
>>                              &pp->children[i].err,