Jeff King <peff@xxxxxxxx> wrote: > 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. Indeed. This also makes me wonder if our output to stdout/stderr suffer from the same theoretical problems if given non-blocking outputs; I suspect they do. >> 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; Yes :) > +int handle_nonblock(FILE *fp, short poll_events) > +{ > + if (ferror(fp) && (errno == EAGAIN || errno == EWOULDBLOCK)) { <snip> > + 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) { <snip> > + } > + funlockfile(fp); > + } while (handle_nonblock(fp, POLLIN)); I haven't used stdio in a while and I'm glad :) Error handling with ferror + clearerr + errno checking is difficult and error-prone. Linus said this back in 2006, too: http://mid.gmane.org/Pine.LNX.4.64.0609141023130.4388@xxxxxxxxxxx So I wonder if we're better off relying entirely on xread/xwrite + strbuf for all our buffering. -- 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