From: Johannes Schindelin Date: Sun, 24 Dec 2006 13:22:48 GMT Subject: Re: [PATCH 7/7] Replace mmap with xmmap, better handling MAP_FAILED. Message-ID: In-Reply-To: <20061224054723.GG8146@spearce.org> Hi, On Sun, 24 Dec 2006, Shawn O. Pearce wrote: > diff --git a/diff.c b/diff.c > index f14288b..244292a 100644 > --- a/diff.c > +++ b/diff.c > @@ -1341,10 +1341,8 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) > fd = open(s->path, O_RDONLY); > if (fd < 0) > goto err_empty; > - s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0); > + s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0); > close(fd); > - if (s->data == MAP_FAILED) > - goto err_empty; The only gripe I have here is that the old code could actually say where the problem occurred ("cannot read data blob for "), but that probably does not matter so much, now that we can hardly run out of memory on a decent machine, even using big packfiles. > diff --git a/read-cache.c b/read-cache.c > index b8d83cc..ca3efbb 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -798,7 +798,7 @@ int read_cache_from(const char *path) > cache_mmap_size = st.st_size; > errno = EINVAL; > if (cache_mmap_size >= sizeof(struct cache_header) + 20) > - cache_mmap = mmap(NULL, cache_mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); > + cache_mmap = xmmap(NULL, cache_mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); > } > close(fd); > if (cache_mmap == MAP_FAILED) This MAP_FAILED no longer has anything to do with MAP_FAILED, but rather with fstat failed, so you probably want to move that into an else construct just before "close(fd);". All in all it is a good change -- for the builtin programs. But it is less good for the libification. Maybe it is time for a discussion about the possible strategies to avoid dying in libgit.a? Ciao, Dscho