Subject: Re: Bug: .gitconfig folder

From: Junio C Hamano <[email-masked]>
Date: Thu, 28 May 2015 10:06:43 -0700

Jeff King <[email-masked]> writes:

> On Wed, May 27, 2015 at 03:38:12PM -0700, Junio C Hamano wrote:
>
>> The patch was meant to be a tongue-in-cheek tangent that is a vast
>> improvement for cases where we absolutely need to use mmap but does
>> not help the OP at all ;-)  I do not think there is any need for the
>> config reader to read the existing file via mmap interface; just
>> open it, strbuf_read() the whole thing (and complain when it cannot)
>> and we should be ok.
>> 
>> Or do we write back through the mmaped region or something?
>
> No, I think we must never do that in our code because our compat mmap
> implementation uses pread(). So all maps must be MAP_PRIVATE (and our
> compat mmap barfs if it is not).
>
> I started to go the strbuf_read() route, but it just felt so dirty to
> change the way the code works only to try to get a better error message.

Hmm.  I actually thought that we long time ago updated the system to
read small loose object files via read(2) instead of mmap(2) purely
as an optimization, as mmap(2) is a bad match if you are going to
read the whole thing from the beginning to the end anyway, and the
"why not strbuf_read() the whole configuration file" was a suggestion
along that line.

But apparently we do not have such an optimization in read_object()
codepath, perhaps I was hallucinating X-<.

> ... but the config-writing code is such a tangled
> mess that I don't want to spend the time or risk the regressions.

That part I agree with.  I was kinda hoping that the previous GSoC
would clean it up, but that did not happen.