sha1_file: fix error message for alternate objects

When we fail to open a corrupt loose object, we report an error and mention the filename via sha1_file_name(). However, that function will always give us a path in the local repository, whereas the corrupt object may have come from an alternate. The result is a very misleading error message. Teach the open_sha1_file() and stat_sha1_file() helpers to pass back the path they found, so that we can report it correctly. Note that the pointers we return go to static storage (e.g., from sha1_file_name()), which is slightly dangerous. However, these helpers are static local helpers, and the names are used for immediately generating error messages. The simplicity is an acceptable tradeoff for the danger. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 13, 2017 at 12:54 UTC 771e7d578ee93753f5ac0ed346effd0af3d5a4b4
2 files changed +41 -15
sha1_file.c
+31 -15
@@ -1613,39 +1613,54 @@ int git_open(const char *name)
1613 }
1614 }
1615
1616 -static int stat_sha1_file(const unsigned char *sha1, struct stat *st)
1616 +/*
1617 + * Find "sha1" as a loose object in the local repository or in an alternate.
1618 + * Returns 0 on success, negative on failure.
1619 + *
1620 + * The "path" out-parameter will give the path of the object we found (if any).
1621 + * Note that it may point to static storage and is only valid until another
1622 + * call to sha1_file_name(), etc.
1623 + */
1624 +static int stat_sha1_file(const unsigned char *sha1, struct stat *st,
1625 + const char **path)
1626 {
1627 struct alternate_object_database *alt;
1628
1620 - if (!lstat(sha1_file_name(sha1), st))
1629 + *path = sha1_file_name(sha1);
1630 + if (!lstat(*path, st))
1631 return 0;
1632
1633 prepare_alt_odb();
1634 errno = ENOENT;
1635 for (alt = alt_odb_list; alt; alt = alt->next) {
1626 - const char *path = alt_sha1_path(alt, sha1);
1627 - if (!lstat(path, st))
1636 + *path = alt_sha1_path(alt, sha1);
1637 + if (!lstat(*path, st))
1638 return 0;
1639 }
1640
1641 return -1;
1642 }
1643
1634 -static int open_sha1_file(const unsigned char *sha1)
1644 +/*
1645 + * Like stat_sha1_file(), but actually open the object and return the
1646 + * descriptor. See the caveats on the "path" parameter above.
1647 + */
1648 +static int open_sha1_file(const unsigned char *sha1, const char **path)
1649 {
1650 int fd;
1651 struct alternate_object_database *alt;
1652 int most_interesting_errno;
1653
1640 - fd = git_open(sha1_file_name(sha1));
1654 + *path = sha1_file_name(sha1);
1655 + fd = git_open(*path);
1656 if (fd >= 0)
1657 return fd;
1658 most_interesting_errno = errno;
1659
1660 prepare_alt_odb();
1661 for (alt = alt_odb_list; alt; alt = alt->next) {
1647 - const char *path = alt_sha1_path(alt, sha1);
1648 - fd = git_open(path);
1662 + *path = alt_sha1_path(alt, sha1);
1663 + fd = git_open(*path);
1664 if (fd >= 0)
1665 return fd;
1666 if (most_interesting_errno == ENOENT)
@@ -1657,10 +1672,11 @@ static int open_sha1_file(const unsigned char *sha1)
1672
1673 void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
1674 {
1675 + const char *path;
1676 void *map;
1677 int fd;
1678
1663 - fd = open_sha1_file(sha1);
1679 + fd = open_sha1_file(sha1, &path);
1680 map = NULL;
1681 if (fd >= 0) {
1682 struct stat st;
@@ -1669,7 +1685,7 @@ void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
1685 *size = xsize_t(st.st_size);
1686 if (!*size) {
1687 /* mmap() is forbidden on empty files */
1672 - error("object file %s is empty", sha1_file_name(sha1));
1688 + error("object file %s is empty", path);
1689 return NULL;
1690 }
1691 map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
@@ -2789,8 +2805,9 @@ static int sha1_loose_object_info(const unsigned char *sha1,
2805 * object even exists.
2806 */
2807 if (!oi->typep && !oi->typename && !oi->sizep) {
2808 + const char *path;
2809 struct stat st;
2793 - if (stat_sha1_file(sha1, &st) < 0)
2810 + if (stat_sha1_file(sha1, &st, &path) < 0)
2811 return -1;
2812 if (oi->disk_sizep)
2813 *oi->disk_sizep = st.st_size;
@@ -2986,6 +3003,8 @@ void *read_sha1_file_extended(const unsigned char *sha1,
3003 {
3004 void *data;
3005 const struct packed_git *p;
3006 + const char *path;
3007 + struct stat st;
3008 const unsigned char *repl = lookup_replace_object_extended(sha1, flag);
3009
3010 errno = 0;
@@ -3001,12 +3020,9 @@ void *read_sha1_file_extended(const unsigned char *sha1,
3020 die("replacement %s not found for %s",
3021 sha1_to_hex(repl), sha1_to_hex(sha1));
3022
3004 - if (has_loose_object(repl)) {
3005 - const char *path = sha1_file_name(sha1);
3006 -
3023 + if (!stat_sha1_file(repl, &st, &path))
3024 die("loose object %s (stored in %s) is corrupt",
3025 sha1_to_hex(repl), path);
3009 - }
3026
3027 if ((p = has_packed_and_bad(repl)) != NULL)
3028 die("packed object %s (stored in %s) is corrupt",
t/t1450-fsck.sh
+10
@@ -550,4 +550,14 @@ test_expect_success 'fsck --name-objects' '
550 )
551 '
552
553 +test_expect_success 'alternate objects are correctly blamed' '
554 + test_when_finished "rm -rf alt.git .git/objects/info/alternates" &&
555 + git init --bare alt.git &&
556 + echo "../../alt.git/objects" >.git/objects/info/alternates &&
557 + mkdir alt.git/objects/12 &&
558 + >alt.git/objects/12/34567890123456789012345678901234567890 &&
559 + test_must_fail git fsck >out 2>&1 &&
560 + grep alt.git out
561 +'
562 +
563 test_done