Skip to content
  • Jeff King's avatar
    avoid "write_in_full(fd, buf, len) != len" pattern · 06f46f23
    Jeff King authored and Junio C Hamano's avatar Junio C Hamano committed
    The return value of write_in_full() is either "-1", or the
    requested number of bytes[1]. If we make a partial write
    before seeing an error, we still return -1, not a partial
    value. This goes back to f6aa66cb (write_in_full: really
    write in full or return error on disk full., 2007-01-11).
    
    So checking anything except "was the return value negative"
    is pointless. And there are a couple of reasons not to do
    so:
    
      1. It can do a funny signed/unsigned comparison. If your
         "len" is signed (e.g., a size_t) then the compiler will
         promote the "-1" to its unsigned variant.
    
         This works out for "!= len" (unless you really were
         trying to write the maximum size_t bytes), but is a
         bug if you check "< len" (an example of which was fixed
         recently in config.c).
    
         We should avoid promoting the mental model that you
         need to check the length at all, so that new sites are
         not tempted to copy us.
    
      2. Checking for a negative value is shorter to type,
         especially when the length is an expression.
    
      3. Linus says so. In d34cf19b
    
     (Clean up write_in_full()
         users, 2007-01-11), right after the write_in_full()
         semantics were changed, he wrote:
    
           I really wish every "write_in_full()" user would just
           check against "<0" now, but this fixes the nasty and
           stupid ones.
    
         Appeals to authority aside, this makes it clear that
         writing it this way does not have an intentional
         benefit. It's a historical curiosity that we never
         bothered to clean up (and which was undoubtedly
         cargo-culted into new sites).
    
    So let's convert these obviously-correct cases (this
    includes write_str_in_full(), which is just a wrapper for
    write_in_full()).
    
    [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.
    
    Signed-off-by: default avatarJeff King <peff@peff.net>
    Reviewed-by: default avatarJonathan Nieder <jrnieder@gmail.com>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    06f46f23