object-file: emit corruption errors when detected

Instead of relying on errno being preserved across function calls, teach do_oid_object_info_extended() to itself report object corruption when it first detects it. There are 3 types of corruption being detected: - when a replacement object is missing - when a loose object is corrupt - when a packed object is corrupt and the object cannot be read in another way Note that in the RHS of this patch's diff, a check for ENOENT that was introduced in 3ba7a06552 (A loose object is not corrupt if it cannot be read due to EMFILE, 2010-10-28) is also removed. The purpose of this check is to avoid a false report of corruption if the errno contains something like EMFILE (or anything that is not ENOENT), in which case a more generic report is presented. Because, as of this patch, we no longer rely on such a heuristic to determine corruption, but surface the error message at the point when we read something that we did not expect, this check is no longer necessary. Besides being more resilient, this also prepares for a future patch in which an indirect caller of do_oid_object_info_extended() will need such functionality. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Dec 14, 2022 at 11:17 UTC 9e59b38c88cdd79964cc92860906a0737bee3797
2 files changed +31 -27
object-file.c
+28 -27
@@ -1422,7 +1422,9 @@ static int loose_object_info(struct repository *r,
1422 struct object_info *oi, int flags)
1423 {
1424 int status = 0;
1425 + int fd;
1426 unsigned long mapsize;
1427 + const char *path;
1428 void *map;
1429 git_zstream stream;
1430 char hdr[MAX_HEADER_LEN];
@@ -1443,7 +1445,6 @@ static int loose_object_info(struct repository *r,
1445 * object even exists.
1446 */
1447 if (!oi->typep && !oi->type_name && !oi->sizep && !oi->contentp) {
1446 - const char *path;
1448 struct stat st;
1449 if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
1450 return quick_has_loose(r, oid) ? 0 : -1;
@@ -1454,7 +1455,13 @@ static int loose_object_info(struct repository *r,
1455 return 0;
1456 }
1457
1457 - map = map_loose_object(r, oid, &mapsize);
1458 + fd = open_loose_object(r, oid, &path);
1459 + if (fd < 0) {
1460 + if (errno != ENOENT)
1461 + error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
1462 + return -1;
1463 + }
1464 + map = map_fd(fd, path, &mapsize);
1465 if (!map)
1466 return -1;
1467
@@ -1492,6 +1499,10 @@ static int loose_object_info(struct repository *r,
1499 break;
1500 }
1501
1502 + if (status && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
1503 + die(_("loose object %s (stored in %s) is corrupt"),
1504 + oid_to_hex(oid), path);
1505 +
1506 git_inflate_end(&stream);
1507 cleanup:
1508 munmap(map, mapsize);
@@ -1601,6 +1612,15 @@ static int do_oid_object_info_extended(struct repository *r,
1612 continue;
1613 }
1614
1615 + if (flags & OBJECT_INFO_DIE_IF_CORRUPT) {
1616 + const struct packed_git *p;
1617 + if ((flags & OBJECT_INFO_LOOKUP_REPLACE) && !oideq(real, oid))
1618 + die(_("replacement %s not found for %s"),
1619 + oid_to_hex(real), oid_to_hex(oid));
1620 + if ((p = has_packed_and_bad(r, real)))
1621 + die(_("packed object %s (stored in %s) is corrupt"),
1622 + oid_to_hex(real), p->pack_name);
1623 + }
1624 return -1;
1625 }
1626
@@ -1653,7 +1673,8 @@ int oid_object_info(struct repository *r,
1673
1674 static void *read_object(struct repository *r,
1675 const struct object_id *oid, enum object_type *type,
1656 - unsigned long *size)
1676 + unsigned long *size,
1677 + int die_if_corrupt)
1678 {
1679 struct object_info oi = OBJECT_INFO_INIT;
1680 void *content;
@@ -1661,7 +1682,8 @@ static void *read_object(struct repository *r,
1682 oi.sizep = size;
1683 oi.contentp = &content;
1684
1664 - if (oid_object_info_extended(r, oid, &oi, 0) < 0)
1685 + if (oid_object_info_extended(r, oid, &oi, die_if_corrupt
1686 + ? OBJECT_INFO_DIE_IF_CORRUPT : 0) < 0)
1687 return NULL;
1688 return content;
1689 }
@@ -1697,35 +1719,14 @@ void *read_object_file_extended(struct repository *r,
1719 int lookup_replace)
1720 {
1721 void *data;
1700 - const struct packed_git *p;
1701 - const char *path;
1702 - struct stat st;
1722 const struct object_id *repl = lookup_replace ?
1723 lookup_replace_object(r, oid) : oid;
1724
1725 errno = 0;
1707 - data = read_object(r, repl, type, size);
1726 + data = read_object(r, repl, type, size, 1);
1727 if (data)
1728 return data;
1729
1711 - obj_read_lock();
1712 - if (errno && errno != ENOENT)
1713 - die_errno(_("failed to read object %s"), oid_to_hex(oid));
1714 -
1715 - /* die if we replaced an object with one that does not exist */
1716 - if (repl != oid)
1717 - die(_("replacement %s not found for %s"),
1718 - oid_to_hex(repl), oid_to_hex(oid));
1719 -
1720 - if (!stat_loose_object(r, repl, &st, &path))
1721 - die(_("loose object %s (stored in %s) is corrupt"),
1722 - oid_to_hex(repl), path);
1723 -
1724 - if ((p = has_packed_and_bad(r, repl)))
1725 - die(_("packed object %s (stored in %s) is corrupt"),
1726 - oid_to_hex(repl), p->pack_name);
1727 - obj_read_unlock();
1728 -
1730 return NULL;
1731 }
1732
@@ -2268,7 +2269,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
2269
2270 if (has_loose_object(oid))
2271 return 0;
2271 - buf = read_object(the_repository, oid, &type, &len);
2272 + buf = read_object(the_repository, oid, &type, &len, 0);
2273 if (!buf)
2274 return error(_("cannot read object for %s"), oid_to_hex(oid));
2275 hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
object-store.h
+3
@@ -445,6 +445,9 @@ struct object_info {
445 */
446 #define OBJECT_INFO_FOR_PREFETCH (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)
447
448 +/* Die if object corruption (not just an object being missing) was detected. */
449 +#define OBJECT_INFO_DIE_IF_CORRUPT 32
450 +
451 int oid_object_info_extended(struct repository *r,
452 const struct object_id *,
453 struct object_info *, unsigned flags);