Re: [PATCH v2 1/9] http: use --stdin when indexing dumb HTTP pack

[Junio C Hamano <gitster@pobox.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

> diff --git a/http.c b/http.c
> index 62aa995245..39cbd56702 100644
> --- a/http.c
> +++ b/http.c
> @@ -2270,9 +2270,9 @@ int finish_http_pack_request(struct http_pack_request *preq)
>  {
>  	struct packed_git **lst;
>  	struct packed_git *p = preq->target;
> -	char *tmp_idx;
> -	size_t len;
>  	struct child_process ip = CHILD_PROCESS_INIT;
> +	int tmpfile_fd;
> +	int ret = 0;
>  
>  	close_pack_index(p);
>  
> @@ -2284,35 +2284,24 @@ int finish_http_pack_request(struct http_pack_request *preq)
>  		lst = &((*lst)->next);
>  	*lst = (*lst)->next;
>  
> -	if (!strip_suffix(preq->tmpfile.buf, ".pack.temp", &len))
> -		BUG("pack tmpfile does not end in .pack.temp?");
> -	tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile.buf);
> +	tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY);

Shouldn't we catch the case where we cannot open the file for
reading and return -1 to have the caller handle the error, just like
the case where other errors are detected in this function?

>  	argv_array_push(&ip.args, "index-pack");
> -	argv_array_pushl(&ip.args, "-o", tmp_idx, NULL);
> -	argv_array_push(&ip.args, preq->tmpfile.buf);
> +	argv_array_push(&ip.args, "--stdin");
>  	ip.git_cmd = 1;
> -	ip.no_stdin = 1;
> +	ip.in = tmpfile_fd;
>  	ip.no_stdout = 1;
>  
>  	if (run_command(&ip)) {
> -		unlink(preq->tmpfile.buf);
> -		unlink(tmp_idx);
> -		free(tmp_idx);
> -		return -1;
> -	}
> -
> -	unlink(sha1_pack_index_name(p->hash));
> -
> -	if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->hash))
> -	 || finalize_object_file(tmp_idx, sha1_pack_index_name(p->hash))) {
> -		free(tmp_idx);
> -		return -1;
> +		ret = -1;
> +		goto cleanup;
>  	}
>  
>  	install_packed_git(the_repository, p);
> -	free(tmp_idx);
> -	return 0;
> +cleanup:
> +	close(tmpfile_fd);
> +	unlink(preq->tmpfile.buf);
> +	return ret;
>  }
>  
>  struct http_pack_request *new_http_pack_request(