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

From: Jeff King <[email-masked]>
Date: Wed, 8 Apr 2015 02:17:13 +0200

On Wed, Apr 08, 2015 at 12:43:09AM +0200, Rasmus Villemoes wrote:

> 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.

Ah, right, that makes sense.

I think it may be OK to just `die()` on ENOMEM here. The "try to free"
routine is basically about unmapping pack memory, in case it is clogging
up the address space (it is mmap'd, so it shouldn't cause real RAM
pressure; the OS can just drop the pages from the cache if it wants).

But getdelim() calls are not likely to be big allocations in the first
place (so they are not likely to fail at all, and if they do, we really
are horribly out of memory). I'd also question whether
release_pack_memory is doing anything in practice on 64-bit machines. We
have quite a bit of address space to work with in that case.

-Peff