Tay Ray Chuan <rctay89@gmail.com> wrote:
quoted text
> > @@ -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.
:-)
 
quoted text
> > +       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.
 
quoted text
> > +       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?

Yup, good point, thanks.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html