Jeff King <peff@xxxxxxxx> 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. -- 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