load_contents(): don't try to mmap an empty file
authorMichael Haggerty <mhagger@alum.mit.edu>
Wed, 24 Jan 2018 11:14:15 +0000 (12:14 +0100)
committerJunio C Hamano <gitster@pobox.com>
Wed, 24 Jan 2018 20:55:26 +0000 (12:55 -0800)
We don't actually create zero-length `packed-refs` files, but they are
valid and we should handle them correctly. The old code `xmmap()`ed
such files, which led to an error when `munmap()` was called. So, if
the `packed-refs` file is empty, leave the snapshot at its zero values
and return 0 without trying to read or mmap the file.

Returning 0 also makes `create_snapshot()` exit early, which avoids
the technically undefined comparison `NULL < NULL`.

Reported-by: Kim Gybels <kgybels@infogroep.be>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs/packed-backend.c
index c2efe912cc834e36ac85561816e3dd34e217c5b8..e30c233970c9f1ad05c3bc2cea42c9edfefa8e5a 100644 (file)
@@ -461,7 +461,8 @@ static void verify_buffer_safe(struct snapshot *snapshot)
 /*
  * Depending on `mmap_strategy`, either mmap or read the contents of
  * the `packed-refs` file into the snapshot. Return 1 if the file
- * existed and was read, or 0 if the file was absent. Die on errors.
+ * existed and was read, or 0 if the file was absent or empty. Die on
+ * errors.
  */
 static int load_contents(struct snapshot *snapshot)
 {
@@ -492,19 +493,17 @@ 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) {
+               return 0;
+       } else if (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->mmapped = 0;
-               break;
-       case MMAP_TEMPORARY:
-       case MMAP_OK:
+       } else {
                snapshot->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
                snapshot->mmapped = 1;
-               break;
        }
        close(fd);