Re: [PATCH v2 5/6] http-fetch: Use index-pack rather than verify-pack to check packs
Tay Ray Chuan <rctay89@gmail.com> wrote:
Show 7 quoted lines
> > @@ -152,7 +152,6 @@ struct http_pack_request
> > struct packed_git *target;
> > struct packed_git **lst;
> > FILE *packfile;
> > - char filename[PATH_MAX];
>
> Why this change? Just curious, nothing strong against it.
Split into a new patch. I'll share my reasons in the commit message.
:-)
Show 5 quoted lines
> > + tmp_idx = xstrdup(preq->tmpfile);
> > + strcpy(tmp_idx + strlen(tmp_idx) - strlen(".pack.temp"),
> > + ".idx.temp");
>
> Could we use a strbuf here?Doesn't seem worth it. I just started trying to rework this with a
strbuf and I just don't see any benefit here. We know the tmpfile
ends with ".pack.temp" when we created this request structure. So
a strdup and overwrite of the tail just works.
Show 5 quoted lines
> > + if (move_temp_to_file(preq->tmpfile, sha1_pack_name(p->sha1))
> > + || move_temp_to_file(tmp_idx, sha1_pack_index_name(p->sha1))) {
>
> Hmm, when moving the pack index file, should we unlink() the old,
> downloaded one first?--
Shawn.
Shawn O. Pearce <spearce@spearce.org>