Re: [PATCH 01/10] strbuf: Add strbuf_read_noblock
- From
Stefan Beller <sbeller@google.com>
- Date
- Sep 17, 2015, 17:45 UTC
- Message-ID
- <CAGZ79kYnZr3nb_5n-5J0vCMi7xb91y-OkrAEq8-uH2PvzmkSmA@mail.gmail.com>
- In-Reply-To
- <20150917173536.GA28987@sigill.intra.peff.net>
On Thu, Sep 17, 2015 at 10:35 AM, Jeff King <peff@peff.net> wrote:
>
Show 6 quoted lines
> > 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.
Show 22 quoted lines
>
> 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.
Show 8 quoted lines
> 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