On Thu, Sep 17, 2015 at 10:35 AM, Jeff King <peff@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html