On Mon, Jun 27, 2016 at 05:49:48PM -0400, Jeff King wrote:
> So in general I would say that handing non-blocking descriptors to git
> is not safe. I think it's possible to loop on getdelim() when we see
> EAGAIN, but I'm not sure if it's worth it.
The patch for that is below, for the curious. It works with even this:
{
for i in H E A D; do
sleep 1
printf $i
done
printf "\n"
} | nonblock git cat-file --batch-check
Note that I folded the "did we see EAGAIN" check into my sub-function
(which is the equivalent of your io_wait). You might want to do that in
the xread() code path, too, as it makes the resulting code there a very
nice:
if (errno == EINTR)
continue;
if (handle_nonblock(fd, POLLIN))
continue;
---
diff --git a/strbuf.c b/strbuf.c
index 1ba600b..2147873 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -452,6 +452,38 @@ int strbuf_getcwd(struct strbuf *sb)
return -1;
}
+int handle_nonblock(FILE *fp, short poll_events)
+{
+ if (ferror(fp) && (errno == EAGAIN || errno == EWOULDBLOCK)) {
+ struct pollfd pfd;
+
+ pfd.events = poll_events;
+ pfd.fd = fileno(fp);
+ poll(&pfd, 1, -1);
+ clearerr(fp);
+ return 1;
+ }
+ return 0;
+}
+
+static int getline_stdio_loop(struct strbuf *sb, FILE *fp, int term)
+{
+ int ch;
+ do {
+ errno = 0;
+ flockfile(fp);
+ while ((ch = getc_unlocked(fp)) != EOF) {
+ if (!strbuf_avail(sb))
+ strbuf_grow(sb, 1);
+ sb->buf[sb->len++] = ch;
+ if (ch == term)
+ break;
+ }
+ funlockfile(fp);
+ } while (handle_nonblock(fp, POLLIN));
+ return ch;
+}
+
#ifdef HAVE_GETDELIM
int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
{
@@ -465,12 +497,31 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int
term)
/* Translate slopbuf to NULL, as we cannot call realloc on it */
if (!sb->alloc)
sb->buf = NULL;
+
+ errno = 0;
r = getdelim(&sb->buf, &sb->alloc, term, fp);
if (r > 0) {
sb->len = r;
- return 0;
}
+
+ /*
+ * getdelim will return characters to us even if it sees EAGAIN;
+ * we want to read all the way to an actual delimiter or EOF.
+ *
+ * We can't just loop on getdelim, though, as it wants to write
+ * the whole buffer itself. So fall back to a stdio loop, which
+ * can incrementally add to our strbuf.
+ */
+ if (handle_nonblock(fp, POLLIN)) {
+ getline_stdio_loop(sb, fp, term);
+ /* Propagate real errors */
+ if (ferror(fp))
+ r = -1;
+ }
+ if (r > 0)
+ return 0;
+
assert(r == -1);
/*
@@ -507,15 +558,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int
term)
return EOF;
strbuf_reset(sb);
- flockfile(fp);
- while ((ch = getc_unlocked(fp)) != EOF) {
- if (!strbuf_avail(sb))
- strbuf_grow(sb, 1);
- sb->buf[sb->len++] = ch;
- if (ch == term)
- break;
- }
- funlockfile(fp);
+ ch = getline_stdio_loop(sb, fp, term);
if (ch == EOF && sb->len == 0)
return EOF;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
