Jeff King <peff@xxxxxxxx> writes:
> I think they have to loop for correctness, but they may do this:
>
> if (xread(fd, buf, len) < 0)
> die_errno("OMG, an error!");
>
> which is not correct if "fd" is unknowingly non-blocking.
> ...
> The spinning behavior is not great, but does mean that we spin and
> continue rather than bailing with an error.
OK, that is a problem (I just read through "git grep xread -- \*.c".
There aren't that many codepaths that read from a fd that we didn't
open ourselves, but there indeed are some.
So it seems that we would want a xread() that spins to help such
codepaths, and xread_nonblock() that gives a short-read upon
EWOULDBLOCK. They can share a helper function, of course.
> If we reset errno to "0" at the top of the function, we could get around
> one problem, but it still makes an annoying interface: the caller has to
> check errno for any 0-return to figure out if it was really EOF, or just
> EAGAIN.
>
> If we return -1, though, we have a similar annoyance. If the caller
> notices a -1 return value and finds EAGAIN, they still may need to check
> sb->len to see if they made forward progress and have data they should
> be dealing with.
OK. Trying to repurpose strbuf_read() for non-blocking FD was a
silly idea to begin with, as it wants to read thru to the EOF, and
setting FD explicitly to O_NONBLOCK is a sign that the caller wants
to grab as much as possible and does not want to wait for the EOF.
So assuming we want strbuf_read_nonblock(), what interface do we
want from it? We could:
* Have it grab as much as possible into sb as long as it does not
block?
* Have it grab reasonably large amount into sb, and not blocking is
an absolute requirement, but the function is not required to read
everything that is available on the FD (i.e. the caller is
expected to loop)?
If we choose the latter, then your "EAGAIN? EOF?" becomes easier,
no? We only have to do a single call to xread(), and then we:
- get EAGAIN or EWOULDBLOCK; leave sb as-is, set errno==EAGAIN and
return -1.
- get something (in which case that is not an EOF yet); append to
sb, return the number of bytes.
- get EOF; leave sb as-is, return 0.
ssize_t strbuf_read_nonblock(struct strbuf *sb, int fd, size_t hint)
{
strbuf_grow(sb, hint ? hint : 8192);
ssize_t want = sb->alloc - sb->len - 1;
ssize_t got = xread_nonblock(fd, sb->buf + sb->len, want);
if (got < 0) {
if (errno == EWOULDBLOCK)
errno = EAGAIN; /* make life easier for the caller */
return got;
}
sb->len += got;
sb->buf[sb->len] = '\0';
return got;
}
--
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