From: Jeff King <peff@peff.net>
Date: Sat, 01 Aug 2026 13:53:13 GMT
Subject: Re: [PATCH v6 2/6] http: avoid closing index-pack input twice
Message-ID: <20260801135313.GA2041176@coredump.intra.peff.net>
In-Reply-To: <28662b0fd892ecf6246be185ccb2d4654fb780a5.1785111375.git.tnyman@openai.com>

On Sun, Jul 26, 2026 at 05:28:39PM -0700, Ted Nyman wrote:

> finish_http_pack_request() passes its staging-file descriptor to
> index-pack through child_process.in. start_command() takes ownership
> of a supplied descriptor and closes it, even when starting the child
> fails.
> 
> Do not close the descriptor again after run_command() returns.

Thanks for splitting this out.

> @@ -2704,13 +2704,8 @@ int finish_http_pack_request(struct http_pack_request *preq)
>  	else
>  		ip.no_stdout = 1;
>  
> -	if (run_command(&ip)) {
> +	if (run_command(&ip))
>  		ret = -1;
> -		goto cleanup;
> -	}
> -
> -cleanup:
> -	close(tmpfile_fd);

The patch _could_ just be a one-liner dropping this close(). Removing
the cleanup label here is optional, but is a simplification that works
because nobody else jumps to it (which must be true because we'd fail to
compile otherwise).

I probably would have mentioned that in the commit message, but I think
there's diminishing returns in trying to polish further.

-Peff