fsck: parse loose object paths directly

When we iterate over the list of loose objects to check, we get the actual path of each object. But we then throw it away and pass just the sha1 to fsck_sha1(), which will do a fresh lookup. Usually it would find the same object, but it may not if an object exists both as a loose and a packed object. We may end up checking the packed object twice, and never look at the loose one. In practice this isn't too terrible, because if fsck doesn't complain, it means you have at least one good copy. But since the point of fsck is to look for corruption, we should be thorough. The new read_loose_object() interface can help us get the data from disk, and then we replace parse_object() with parse_object_buffer(). As a bonus, our error messages now mention the path to a corrupted object, which should make it easier to track down errors when they do happen. 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:59 UTC c68b489e56431cf27f7719913ab09ddc62f95912
2 files changed +49 -13
builtin/fsck.c
+33 -13
@@ -362,18 +362,6 @@ static int fsck_obj(struct object *obj)
362 return 0;
363 }
364
365 -static int fsck_sha1(const unsigned char *sha1)
366 -{
367 - struct object *obj = parse_object(sha1);
368 - if (!obj) {
369 - errors_found |= ERROR_OBJECT;
370 - return error("%s: object corrupt or missing",
371 - sha1_to_hex(sha1));
372 - }
373 - obj->flags |= HAS_OBJ;
374 - return fsck_obj(obj);
375 -}
376 -
365 static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
366 unsigned long size, void *buffer, int *eaten)
367 {
@@ -488,9 +476,41 @@ static void get_default_heads(void)
476 }
477 }
478
479 +static struct object *parse_loose_object(const unsigned char *sha1,
480 + const char *path)
481 +{
482 + struct object *obj;
483 + void *contents;
484 + enum object_type type;
485 + unsigned long size;
486 + int eaten;
487 +
488 + if (read_loose_object(path, sha1, &type, &size, &contents) < 0)
489 + return NULL;
490 +
491 + if (!contents && type != OBJ_BLOB)
492 + die("BUG: read_loose_object streamed a non-blob");
493 +
494 + obj = parse_object_buffer(sha1, type, size, contents, &eaten);
495 +
496 + if (!eaten)
497 + free(contents);
498 + return obj;
499 +}
500 +
501 static int fsck_loose(const unsigned char *sha1, const char *path, void *data)
502 {
493 - if (fsck_sha1(sha1))
503 + struct object *obj = parse_loose_object(sha1, path);
504 +
505 + if (!obj) {
506 + errors_found |= ERROR_OBJECT;
507 + error("%s: object corrupt or missing: %s",
508 + sha1_to_hex(sha1), path);
509 + return 0; /* keep checking other objects */
510 + }
511 +
512 + obj->flags = HAS_OBJ;
513 + if (fsck_obj(obj))
514 errors_found |= ERROR_OBJECT;
515 return 0;
516 }
t/t1450-fsck.sh
+16
@@ -581,4 +581,20 @@ test_expect_success 'fsck errors in packed objects' '
581 ! grep corrupt out
582 '
583
584 +test_expect_success 'fsck finds problems in duplicate loose objects' '
585 + rm -rf broken-duplicate &&
586 + git init broken-duplicate &&
587 + (
588 + cd broken-duplicate &&
589 + test_commit duplicate &&
590 + # no "-d" here, so we end up with duplicates
591 + git repack &&
592 + # now corrupt the loose copy
593 + file=$(sha1_file "$(git rev-parse HEAD)") &&
594 + rm "$file" &&
595 + echo broken >"$file" &&
596 + test_must_fail git fsck
597 + )
598 +'
599 +
600 test_done