load_contents(): don't try to mmap an empty file

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>

Michael Haggerty committed Jan 24, 2018 at 12:14 UTC 01caf20d57aea73e67337ba1d396dd80a76d9dc3
1 file changed +6 -7
refs/packed-backend.c
+6 -7
@@ -461,7 +461,8 @@ static void verify_buffer_safe(struct snapshot *snapshot)
461 /*
462 * Depending on `mmap_strategy`, either mmap or read the contents of
463 * the `packed-refs` file into the snapshot. Return 1 if the file
464 - * existed and was read, or 0 if the file was absent. Die on errors.
464 + * existed and was read, or 0 if the file was absent or empty. Die on
465 + * errors.
466 */
467 static int load_contents(struct snapshot *snapshot)
468 {
@@ -492,19 +493,17 @@ static int load_contents(struct snapshot *snapshot)
493 die_errno("couldn't stat %s", snapshot->refs->path);
494 size = xsize_t(st.st_size);
495
495 - switch (mmap_strategy) {
496 - case MMAP_NONE:
496 + if (!size) {
497 + return 0;
498 + } else if (mmap_strategy == MMAP_NONE) {
499 snapshot->buf = xmalloc(size);
500 bytes_read = read_in_full(fd, snapshot->buf, size);
501 if (bytes_read < 0 || bytes_read != size)
502 die_errno("couldn't read %s", snapshot->refs->path);
503 snapshot->mmapped = 0;
502 - break;
503 - case MMAP_TEMPORARY:
504 - case MMAP_OK:
504 + } else {
505 snapshot->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
506 snapshot->mmapped = 1;
507 - break;
507 }
508 close(fd);
509