On Fri, Jun 24, 2016 at 12:06:59PM -0700, Junio C Hamano wrote: > > + if (args->time > 077777777777UL) { > > + strbuf_append_ext_header_uint(&ext_header, "mtime", > > + args->time); > > + args->time = 077777777777UL; > > + } > > + > > + if (!ext_header.len) > > + return 0; > > Another symbolic constant to explain this, e.g. TAR_TIME_LIMIT, may > want to exist. This one at least appears twice. I think one of the reasons I am slightly resistant to a symbolic constant is that it tempts people to think that it's OK to change it. It's not. These values are mandated by POSIX, and must match the size of the ustar header field. So the least-repetitive thing would be to define it as: (1UL << (1 + (3 * (sizeof(ustar_header.mtime) - 1)))) - 1 That's pretty horrible to read, but if wrapped in a symbolic constant, at least people would think twice before touching it. ;) -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