Re: [PATCH 4/8] strbuf: add strbuf_read_once to read without blocking

[Stefan Beller <sbeller@google.com>]

On Mon, Dec 14, 2015 at 3:16 PM, Eric Sunshine <ericsunshine@gmail.com> wrote:
> On Mon, Dec 14, 2015 at 2:37 PM, Stefan Beller <sbeller@google.com> wrote:
>> The new call will read from a file descriptor into a strbuf once. The
>> underlying call xread_nonblock is meant to execute without blocking if
>> the file descriptor is set to O_NONBLOCK. It is a bug to call
>> strbuf_read_once on a file descriptor which would block.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git a/strbuf.h b/strbuf.h
>> @@ -367,6 +367,14 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
>>  extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
>>
>>  /**
>> + * Returns the number of new bytes appended to the sb.
>> + * Negative return value signals there was an error returned from
>> + * underlying read(2), in which case the caller should check errno.
>> + * e.g. errno == EAGAIN when the read may have blocked.
>> + */
>> +extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);
>
> strbuf_read_once() is a rather opaque name; without reading the
> documentation, it's difficult to figure out what it means. I wonder if
> strbuf_read_nonblock() or something would be clearer?

Well the underlying read call can block if the fd is not marked for nonblocking,
so I would not name it _nonblock.
I just realize this same argument would make the naming moot for the previous
patch though. (xread_nonblock may block if not marked unblocking)

Currently we really want is a read once and do not try to grab as much
as possible,
but just return quickly. We do not do the non blocking setup, as we deemed that
unneeded because of the preceding poll in a higher layer to signal we have data
there to read.

Junio suggested in the discussion [4/8] maybe we can just use xread
and strbuf_read
in this series, so I am tempted to drop patches {3,4}/8 as that makes
sense to me.
For non blocking stuff we can later re introduce those helper functions though.