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

From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Oct 6, 2025, 12:59 UTC
Message-ID: <87v7ks424z.fsf@collabora.com>
In-Reply-To: <aN4c6l7gRi4auss1@pks.im>

Hi Patrick and thanks for review! I'll fix in v2 all the issues you pointed out.

On Thu, 02 Oct 2025, Patrick Steinhardt <ps@pks.im> wrote:

Show 11 quoted lines

>> +		 * 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.