Re: [PATCH 3/6] strbuf_getwholeline: use getc_unlocked

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 07 2015, Jeff King <peff@xxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]