On Tue, Apr 07 2015, Jeff King <p...@peff.net> wrote: > On Tue, Apr 07, 2015 at 03:48:33PM +0200, Rasmus Villemoes wrote: > >> Implementation-wise, I think strbuf_getwholeline could be implemented >> mostly as a simple wrapper for getdelim. If I'm reading the current code >> and the posix spec for getdelim correctly, something like this should do >> it (though obviously not meant to be included as-is): > > I think it's close to what we want. strbuf_grow calls xrealloc, which > will call try_to_free_routine() and possibly die() for us. So we would > probably want to check errno on failure and respond similarly if we get > ENOMEM.
Hm, I'm afraid it's not that simple. It seems that data may be lost from the stream if getdelim encounters ENOMEM: Looking at the glibc implementation (libio/iogetdelim.c), if reallocating the user buffer fails, -1 is returned (presumably with errno==ENOMEM set by realloc), and there's no way of knowing how many bytes were already copied to the buffer (cur_len). For regular files, I suppose one could do a ftell/fseek dance. For other cases, I don't see a reliable way to retry upon ENOMEM. Related thread on the posix mailing list: http://thread.gmane.org/gmane.comp.standards.posix.austin.general/10091 > I wonder if it is even worth doing the getc_unlocked dance at all. It > would potentially speed up the fallback code, but my hope that would be > that most systems would simply use the getdelim() version. Well, it's not really an intrusive patch, and 42% speedup is rather significant. And, of course, the above ENOMEM issue may mean that getdelim isn't usable after all. Rasmus -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
