Hi, On Sat, 13 Jan 2018, Kim Gybels wrote: > Take a hint from commit ea68b0ce9f8ce8da3e360aed3cbd6720159ffbee and use Maybe use ea68b0ce9f8 (hash-object: don't use mmap() for small files, 2010-02-21) instead of the full commit name? > read() instead of mmap() for small packed-refs files. > > This also fixes the problem[1] where xmmap() returns NULL for zero > length[2], for which munmap() later fails. > > Alternatively, we could simply check for NULL before munmap(), or > introduce an xmunmap() that could be used together with xmmap(). > > [1] https://github.com/git-for-windows/git/issues/1410 > [2] Logic introduced in commit 9130ac1e1966adb9922e64f645730d0d45383495 > > Signed-off-by: Kim Gybels <kgybels@xxxxxxxxxxxx> > --- > refs/packed-backend.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/refs/packed-backend.c b/refs/packed-backend.c > index dab8a85d9a..7177e5bc2f 100644 > --- a/refs/packed-backend.c > +++ b/refs/packed-backend.c > @@ -455,6 +455,8 @@ static void verify_buffer_safe(struct snapshot *snapshot) > last_line, eof - last_line); > } > > +#define SMALL_FILE_SIZE (32*1024) > + > /* > * Depending on `mmap_strategy`, either mmap or read the contents of > * the `packed-refs` file into the snapshot. Return 1 if the file > @@ -489,21 +491,21 @@ static int load_contents(struct snapshot *snapshot) > die_errno("couldn't stat %s", snapshot->refs->path); > size = xsize_t(st.st_size); > > - switch (mmap_strategy) { > - case MMAP_NONE: > + if (!size) { > + snapshot->buf = NULL; > + snapshot->eof = NULL; > + snapshot->mmapped = 0; > + } else if (size <= SMALL_FILE_SIZE || mmap_strategy == MMAP_NONE) { > snapshot->buf = xmalloc(size); > bytes_read = read_in_full(fd, snapshot->buf, size); > if (bytes_read < 0 || bytes_read != size) > die_errno("couldn't read %s", snapshot->refs->path); > snapshot->eof = snapshot->buf + size; > snapshot->mmapped = 0; > - break; > - case MMAP_TEMPORARY: > - case MMAP_OK: > + } else { > snapshot->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); > snapshot->eof = snapshot->buf + size; > snapshot->mmapped = 1; > - break; > } > close(fd); Nicely explained, and nicely solved, for a potential extra performance benefit ;-) Thank you! Dscho
