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

> Patrick Steinhardt <ps@pks.im> writes: 
> 
>>> +	/* Buffer stdin for each pipe. */ +	for (int i = 0; i 
>>> < opts->processes; i++) { 
>> 
>> `opts->processes` is of type `size_t`, so let's use the same 
>> type as iterator. 
> 
> Good eyes. 
> 
>>> +		/** 
>> 
>> Nit: multi-line comments should start with "/*", not 
>> "/**". This is also present in multiple other  
> 
> True. 
> 
> Especially for a comment about a specific piece of code and not 
> about an interface---even in a future where we use some tool to 
> extract them, we would not place them in documentation. 
> 
>>> +		 * 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. 
> 
> Good eyes.  Does -1 also have special meaning or we have no risk 
> mistaking this proc->in that was once used with a request to 
> open a pipe?  Regardless, I agree 0 would be a bad choice here. 
>