Subject: Re: [PATCH 01/10] run-command: add stdin callback for parallelization

From: Adrian Ratiu <[email-masked]>
Date: Tue, 14 Oct 2025 20:35:14 +0300

Hi again Patrick and Junio,

On Mon, 06 Oct 2025, Adrian Ratiu <[email-masked]> 
wrote:
> Hi Patrick and thanks for review! I'll fix in v2 all the issues 
> you pointed out. 
> 
> On Thu, 02 Oct 2025, Patrick Steinhardt <[email-masked]> wrote: 
>>> +		 * child input is provided via path_to_stdin when 
>>> the feed_pipe cb is +		 * missing, so we just 
>>> signal an EOF.  +		 */ +		if 
>>> (!opts->feed_pipe) { +			close(proc->in); + 
>>> proc->in = 0;  
>>  Hm. It's curious that we use a valid file descriptor 
>> here. Shouldn't we rather use `-1`? Otherwise I could see that 
>> we might try to close this seemingly valid file descriptor at a 
>> later point in time.   
> 
> I actually asked myself this while preparing the patches, since 
> -1  is a better fit. 
> 
> I only left = 0 for historical reasons, to not modify these 
> patches too much. :) However I do 100% agree with both you and 
> Junio that -1 should be used here. 
> 
> Will do in v2. 

This is much harder and riskier than I originally anticipated, so 
I gave up trying to implement it after a few failed attempts.

In a nutshell, we have to change the < 0, 0 and > 0 semantics 
defined in run-command.h for .in, .out, and .err fds across the 
entire source tree.

It's a massive, error-prone and out-of-scope amount of work.

Can we please just keep the current run-command API which uses 0 
for "no fd passed"? I'd very much like to avoid changing this 
run-command API.