On Wed, Apr 27, 2016 at 04:37:28PM -0400, Jeff King wrote: > > > But anything writing a _new_ refname (whether the actual ref, or > > > referencing it via a symref) should be using check_refname_format() > > > before writing. > > > > Unfortunately, neither check is lesser -- refname_is_safe allows > > refs/heads//foo but not a/b while check_refname_format allows a/b but > > not refs/heads//foo. So sometimes we need both, while other times we > > just need one :( > > IMHO, that sounds like a bug. check_refname_format() should > conceptually[1] be a superset of refname_is_safe(). Is there a case > where we would want to _allow_ a refname that is not safe to look at on > disk? BTW, if there isn't a superset relationship here, then I suspect I may have introduced some loosening or inconsistencies in 03afcbe (read_packed_refs: avoid double-checking sane refs, 2015-04-16). So any tightening now may simply be fixing that (which doesn't change anything with respect to correctness, but may give people more confidence that the tightening is not breaking something people have been depending on). -Peff PS Reading over that commit message, I think it mis-states "refname_is_safe() can only be true if check_refname_format() also failed". It should be "!refname_is_safe() ...". I.e., the condition can only trigger if... -- 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