Re: [PATCH 01/10] strbuf: Add strbuf_read_noblock

[Stefan Beller <sbeller@google.com>]

On Thu, Sep 17, 2015 at 10:35 AM, Jeff King <peff@peff.net> wrote:
>

>
> You _can_ loop on read until you hit EAGAIN, but I think in general you
> shouldn't; if you get a lot of input on this fd, you'll starve all of
> the other descriptors you're polling.  You're better off to read a
> finite amount from each descriptor, and then check again who is ready to
> read.

That's what I do with the current implementation. Except it's not as clear and
concise as I patched it into the strbuf_read.

>
> And then the return value becomes a no-brainer, because it's just the
> return value of read(). Either you got some data, you got EOF, or you
> get an error, which might be EAGAIN. You never have the case where you
> got some data, but then you also got EOF and EAGAIN, and the caller has
> to figure out which.
>
> So I think you really want something like:
>
>   ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
>   {
>         ssize_t cnt;
>
>         strbuf_grow(hint ? hint : 8192);
>         cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
>         if (cnt > 0)
>                 strbuf_setlen(sb->len + cnt);
>         return cnt;
>   }
>
> (where I'm assuming xread passes us back EAGAIN; we could also replace
> it with read and loop on EINTR ourselves).

Yeah that's exactly what I am looking for (the hint may even be over
engineered here, as I have no clue how much data comes back).

So I guess I could just use that new method now.


> I actually wonder if callers who are _expecting_ non-blocking want to
> loop in strbuf_read() at all.
>
> strbuf_read() is really about reading to EOF, and growing the buffer to
> fit all of the input. But that's not at all what you want to do with
> non-blocking. There you believe for some reason that data may be
> available (probably due to poll), and you want to read one chunk of it,
> maybe act, and then go back to polling.

As a micro project (leftover bit maybe?):
When strbuf_read is reading data out from a non blocking pipe, we're currently
spinning (with the EAGAIN/EWOULDBLOCK). Introduce a call to poll
to reduce the spinning.

>
> -Peff