On Sat, Aug 11, 2018 at 05:39:27PM +0200, René Scharfe wrote: > The char array named "buffer" is unlikely to contain a NUL character, so > printing its contents using %s in a die() format is unsafe. Clang's > ASan reports running over the end of buffer in the recently added > skiplist tests in t5504-fetch-receive-strict.sh as a result. > > Use an idiomatic strbuf_getline() loop instead, which ensures the buffer > is always NUL-terminated. As a side-effect this also adds support for > skiplist files with CRLF line endings. Nice. Two other side effects, both of which I think are good: - this is likely faster for a large skiplist due to fewer syscalls - this will no longer complain about a sha1 with a missing newline at the end-of-file (it will quietly handle it as if the newline were there) And one I'm not sure about: - a read() error will now be quietly ignored; I guess we'd have to check ferror(fp) to cover this. I'm not sure if it matters. > fsck.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) Code itself looks good to me (assuming we don't care about the read() error thing). -Peff
