On Tue, Jun 21, 2016 at 11:59:20AM -0400, Jeff King wrote: > > > + if (ustar_size(size) != size) > > > + strbuf_append_ext_header_uint(&ext_header, "size", size); > > > > It needs "S_ISREG(mode) && " as well, no? In practice it probably doesn't > > matter (until someone stores a 8GB long symlink target), but the size field > > should only be set for regular files. > > Thanks for noticing that. I remembered wondering that when I was early > in debugging/diagnosing, but forgot to follow up on it. I agree it's > unlikely in practice, but we should have consistent checks (I think it > would actually make sense to move the ISREG check inside ustar_size, and > then we can apply it consistently here and when generating the header; > my goal with ustar_size() was to avoid having the same logic in multiple > places). Actually, scratch that. It makes things awkward because it would be hard to tell when ustar_size() returned zero because it's a file with a big size, and thus needs a pax header, versus that it was not a file, and therefore must _not_ have a pax header. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html