Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

From: Jeff King <peff@peff.net>
Date: Nov 5, 2015, 06:51 UTC
Message-ID: <20151105065111.GA4725@sigill.intra.peff.net>
In-Reply-To: <xmqqvb9h8ale.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 04, 2015 at 06:05:17PM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> I've always assumed that the original reason why we wanted to set
> the fd to nonblock was because poll(2) only tells us there is
> something to read (even a single byte), and the xread_nonblock()
> call strbuf_read_once() makes with the default size of 8KB is
> allowed to consume all available bytes and then get stuck waiting
> for the remainder of 8KB before returning.
> 
> If the read(2) in xread_nonblock() always returns as soon as we
> receive as much as there is data available without waiting for any
> more, ignoring the size of the buffer (rather, taking the size of
> the buffer only as the upper bound), then there is no need for
> nonblock anywhere.

This latter paragraph was my impression of how pipe reading generally worked, for blocking or non-blocking. That is, if there is data, both cases return what we have (up to the length specified by the user), and it is only when there is _no_ data that we might choose to block.

It's easy to verify experimentally. E.g.:

  perl -e 'while(1) { syswrite(STDOUT, "a", 1); sleep(1); }' |
  strace perl -e 'while(1) { sysread(STDIN, my $buf, 1024) }'

should show a series of 1-byte reads. But of course that only shows that it works on my system[1], not everywhere.

POSIX implies it is the case in the definition of read[2] in two ways:

  1. The O_NONBLOCK behavior for pipes is mentioned only when dealing
     with empty pipes.

  2. Later, it says:

       The value returned may be less than nbyte if the number of bytes
       left in the file is less than nbyte, if the read() request was
       interrupted by a signal, or if the file is a pipe or FIFO or
       special file and has fewer than nbyte bytes immediately available
       for reading.

     That is not explicit, but the "immediately" there seems to imply
     it.

> So perhaps the original reasoning of doing nonblock was faulty, you
> are saying?

Exactly. And therefore a convenient way to deal with the portability issue is to get rid of it. :)

-Peff