On Wed, Sep 13, 2017 at 02:14:30PM -0700, Jonathan Nieder wrote: > > I really wish every "write_in_full()" user would just > > check against "<0" now, but this fixes the nasty and > > stupid ones. > > Ok, you convinced me. > > Should we add a comment to cache.h as well encouraging this?
I'd be OK with a comment, though I don't know that it's strictly necessary. It looks like most of it was just cargo-culted, so removing the offending examples is sufficient. > > [1] A careful reader may notice there is one way that > > write_in_full() can return a different value. If we ask > > write() to write N bytes and get a return value that is > > _larger_ than N, we could return a larger total. But > > besides the fact that this would imply a totally broken > > version of write(), it would already invoke undefined > > behavior. Our internal remaining counter is an unsigned > > size_t, which means that subtracting too many byte will > > wrap it around to a very large number. So we'll instantly > > begin reading off the end of the buffer, trying to write > > gigabytes (or petabytes) of data. > > This footnote just leaves me more confused, since as you mention, > write() never would return a value greater than N. Are you saying we > need to defend against a broken platform where that isn't true? No, I'm saying that my claim that write_in_full() can only return two values (-1 and the original length) is not strictly true. But that it doesn't matter in practice. I don't think we need to defend against such a broken platform, but I didn't want anybody reading the claim to say "aha, you forgot this case". It is a case that does not matter. -Peff
