Re: [PATCH v2 1/9] http: use --stdin when indexing dumb HTTP pack
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 11, 2020, 01:10 UTC
- Message-ID
- <xmqqsgf2csps.fsf@gitster.c.googlers.com>
- In-Reply-To
- <d3b27394cad951a9f1412a04572439513d7e9f4d.1591821067.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 24 quoted lines
> 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?
Show 36 quoted lines
> 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(