Re: [PATCH 3/7] avoid "write_in_full(fd, buf, len) != len" pattern

From: Jeff King <peff@peff.net>
Date: Sep 15, 2017, 00:42 UTC
Message-ID: <20170915004237.wjys6tgyxk6j43op@sigill.intra.peff.net>
In-Reply-To: <20170913211430.GK27425@aiede.mtv.corp.google.com>

On Wed, Sep 13, 2017 at 02:14:30PM -0700, Jonathan Nieder wrote:

Show 7 quoted lines

> >        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.

Show 15 quoted lines

> > [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