On Wed, Sep 27, 2017 at 04:06:22PM +0900, Junio C Hamano wrote: > A few tangents I noticed: > > - the result of readlink should be checked with starts_with() in > the modern codebase (#leftoverbits).
Yes, though it needs to NUL-terminate, too (readlink does not do so automatically). Again, we seem to have left room for the NUL but didn't bother to put it there. > - buffer[256] would mean that we cannot have a branch whose name is > more than a couple of hundred bytes long; as you said, we may be > better off using strbuf_read to read the whole thing. Heh, I almost talked about this in the cover letter, but didn't want to go off on a tangent. But since you mention it... I had the same concern, but actually truncation is not a problem here (for a symlink or a symref). We are only seeing if the contents look vaguely correct, so really we never parse past "refs/" in either case. The real symref resolution happens in refs/files-backend.c these days, and does use a strbuf. -Peff
