{"thread":{"id":"1064","subject":"[PATCH 1/2] verify-pack updates.","startedAt":"2005-07-01T00:15:39Z","lastAt":"2005-07-01T00:15:39Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"5502","messageId":"7vr7ejz8ic.fsf@assigned-by-dhcp.cox.net","threadId":"1064","inReplyTo":null,"subject":"[PATCH 1/2] verify-pack updates.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-07-01T00:15:39Z","receivedAt":"2005-07-01T00:15:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nico pointed out that having verify_pack.c and verify-pack.c was\nconfusing.  Rename verify_pack.c to pack-check.c as suggested,\nand enhances the verification done quite a bit.\n\n - Built-in sha1_file unpacking knows that a base object of a\n   deltified object _must_ be in the same pack, and takes\n   advantage of that fact.\n\n - Earlier verify-pack command only checked the SHA1 sum for the\n   entire pack file and did not look into its contents.  It now\n   checks everything idx file claims to have unpacks correctly.\n\n - It now has a hook to give more detailed information for\n   objects contained in the pack under -v flag.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n*** I used \"git format-patch -M20\" to generate diff to show this\n*** commit as a rename/copy patch.  Let's see if git-apply can\n*** still grok it ;-).\n\n Makefile      |    2 +-\n cache.h       |    9 ++++++++\n fsck-cache.c  |    2 +-\n pack-check.c  |   68 +++++++++++++++++++++++++++++++++++++++++++++++++--------\n pack.h        |    2 +-\n sha1_file.c   |   66 +++++++++++++++++++++++++++++++++++++------------------\n verify-pack.c |   51 ++++++++++++++++++++++++++++++++++---------\n 7 files changed, 156 insertions(+), 44 deletions(-)\n rename verify_pack.c => pack-check.c (25%)\n\ncf43fb6e9fec205d4786a284e1656b3612c28a77\ndiff --git a/Makefile b/Makefile\n--- a/Makefile\n+++ b/Makefile\n@@ -46,7 +46,7 @@ install: $(PROG) $(SCRIPTS)\n \n LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \\\n \t tag.o date.o index.o diff-delta.o patch-delta.o entry.o \\\n-\t epoch.o refs.o csum-file.o verify_pack.o pkt-line.o\n+\t epoch.o refs.o csum-file.o pack-check.o pkt-line.o\n LIB_FILE=libgit.a\n LIB_H=cache.h object.h blob.h tree.h commit.h tag.h delta.h epoch.h csum-file.h \\\n \tpack.h pkt-line.h\ndiff --git a/cache.h b/cache.h\n--- a/cache.h\n+++ b/cache.h\n@@ -251,11 +251,20 @@ extern struct packed_git {\n \tunsigned int pack_use_cnt;\n \tchar pack_name[0]; /* something like \".git/objects/pack/xxxxx.pack\" */\n } *packed_git;\n+\n+struct pack_entry {\n+\tunsigned int offset;\n+\tunsigned char sha1[20];\n+\tstruct packed_git *p;\n+};\n+\n extern void prepare_packed_git(void);\n extern int use_packed_git(struct packed_git *);\n extern void unuse_packed_git(struct packed_git *);\n extern struct packed_git *add_packed_git(char *, int);\n extern int num_packed_objects(const struct packed_git *p);\n extern int nth_packed_object_sha1(const struct packed_git *, int, unsigned char*);\n+extern int find_pack_entry_one(const unsigned char *, struct pack_entry *, struct packed_git *);\n+extern void *unpack_entry_gently(struct pack_entry *, char *, unsigned long *);\n \n #endif /* CACHE_H */\ndiff --git a/fsck-cache.c b/fsck-cache.c\n--- a/fsck-cache.c\n+++ b/fsck-cache.c\n@@ -440,7 +440,7 @@ int main(int argc, char **argv)\n \t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next)\n \t\t\t/* verify gives error messages itself */\n-\t\t\tverify_pack(p); \n+\t\t\tverify_pack(p, 0);\n \n \t\tfor (p = packed_git; p; p = p->next) {\n \t\t\tint num = num_packed_objects(p);\ndiff --git a/verify_pack.c b/pack-check.c\nsimilarity index 25%\nrename from verify_pack.c\nrename to pack-check.c\n--- a/verify_pack.c\n+++ b/pack-check.c\n@@ -10,8 +10,9 @@ static int verify_packfile(struct packed\n \tunsigned long pack_size = p->pack_size;\n \tvoid *pack_base;\n \tstruct pack_header *hdr;\n-\tint nr_objects;\n+\tint nr_objects, err, i;\n \n+\t/* Header consistency check */\n \thdr = p->pack_base;\n \tif (hdr->hdr_signature != htonl(PACK_SIGNATURE))\n \t\treturn error(\"Packfile signature mismatch\", p->pack_name);\n@@ -34,11 +35,47 @@ static int verify_packfile(struct packed\n \tif (memcmp(sha1, pack_base + pack_size - 20, 20))\n \t\treturn error(\"Packfile %s SHA1 mismatch with itself\",\n \t\t\t     p->pack_name);\n-\treturn 0;\n+\n+\t/* Make sure everything reachable from idx is valid.  Since we\n+\t * have verified that nr_objects matches between idx and pack,\n+\t * we do not do scan-streaming check on the pack file.\n+\t */\n+\tfor (i = err = 0; i < nr_objects; i++) {\n+\t\tunsigned char sha1[20];\n+\t\tstruct pack_entry e;\n+\t\tvoid *data;\n+\t\tchar type[20];\n+\t\tunsigned long size;\n+\n+\t\tif (nth_packed_object_sha1(p, i, sha1))\n+\t\t\tdie(\"internal error pack-check nth-packed-object\");\n+\t\tif (!find_pack_entry_one(sha1, &e, p))\n+\t\t\tdie(\"internal error pack-check find-pack-entry-one\");\n+\t\tdata = unpack_entry_gently(&e, type, &size);\n+\t\tif (!data) {\n+\t\t\terr = error(\"cannot unpack %s from %s\",\n+\t\t\t\t    sha1_to_hex(sha1), p->pack_name);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (check_sha1_signature(sha1, data, size, type)) {\n+\t\t\terr = error(\"cannot packed %s from %s corrupt\",\n+\t\t\t\t    sha1_to_hex(sha1), p->pack_name);\n+\t\t\tfree(data);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfree(data);\n+\t}\n+\n+\treturn err;\n }\n \n \n-int verify_pack(struct packed_git *p)\n+static void show_pack_info(struct packed_git *p)\n+{\n+\t/* Next round */\n+}\n+\n+int verify_pack(struct packed_git *p, int verbose)\n {\n \tunsigned long index_size = p->index_size;\n \tvoid *index_base = p->index_base;\n@@ -46,17 +83,30 @@ int verify_pack(struct packed_git *p)\n \tunsigned char sha1[20];\n \tint ret;\n \n+\tret = 0;\n \t/* Verify SHA1 sum of the index file */\n \tSHA1_Init(&ctx);\n \tSHA1_Update(&ctx, index_base, index_size - 20);\n \tSHA1_Final(sha1, &ctx);\n \tif (memcmp(sha1, index_base + index_size - 20, 20))\n-\t\treturn error(\"Packfile index for %s SHA1 mismatch\",\n-\t\t\t     p->pack_name);\n+\t\tret = error(\"Packfile index for %s SHA1 mismatch\",\n+\t\t\t    p->pack_name);\n+\n+\tif (!ret) {\n+\t\t/* Verify pack file */\n+\t\tuse_packed_git(p);\n+\t\tret = verify_packfile(p);\n+\t\tunuse_packed_git(p);\n+\t}\n+\n+\tif (verbose) {\n+\t\tif (ret)\n+\t\t\tprintf(\"%s: bad\\n\", p->pack_name);\n+\t\telse {\n+\t\t\tshow_pack_info(p);\n+\t\t\tprintf(\"%s: ok\\n\", p->pack_name);\n+\t\t}\n+\t}\n \n-\t/* Verify pack file */\n-\tuse_packed_git(p);\n-\tret = verify_packfile(p);\n-\tunuse_packed_git(p);\n \treturn ret;\n }\ndiff --git a/pack.h b/pack.h\n--- a/pack.h\n+++ b/pack.h\n@@ -27,6 +27,6 @@ struct pack_header {\n \tunsigned int hdr_entries;\n };\n \n-extern int verify_pack(struct packed_git *);\n+extern int verify_pack(struct packed_git *, int);\n \n #endif\ndiff --git a/sha1_file.c b/sha1_file.c\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -272,12 +272,6 @@ static int pack_used_ctr;\n static unsigned long pack_mapped;\n struct packed_git *packed_git;\n \n-struct pack_entry {\n-\tunsigned int offset;\n-\tunsigned char sha1[20];\n-\tstruct packed_git *p;\n-};\n-\n static int check_packed_git_idx(const char *path, unsigned long *idx_size_,\n \t\t\t\tvoid **idx_map_)\n {\n@@ -618,22 +612,34 @@ void * unpack_sha1_file(void *map, unsig\n \treturn unpack_sha1_rest(&stream, hdr, *size);\n }\n \n+/* forward declaration for a mutually recursive function */\n+static int packed_object_info(struct pack_entry *entry,\n+\t\t\t      char *type, unsigned long *sizep);\n+\n static int packed_delta_info(unsigned char *base_sha1,\n \t\t\t     unsigned long delta_size,\n \t\t\t     unsigned long left,\n \t\t\t     char *type,\n-\t\t\t     unsigned long *sizep)\n+\t\t\t     unsigned long *sizep,\n+\t\t\t     struct packed_git *p)\n {\n+\tstruct pack_entry base_ent;\n+\n \tif (left < 20)\n \t\tdie(\"truncated pack file\");\n \n+\t/* The base entry _must_ be in the same pack */\n+\tif (!find_pack_entry_one(base_sha1, &base_ent, p))\n+\t\tdie(\"failed to find delta-pack base object %s\",\n+\t\t    sha1_to_hex(base_sha1));\n+\n \t/* We choose to only get the type of the base object and\n \t * ignore potentially corrupt pack file that expects the delta\n \t * based on a base with a wrong size.  This saves tons of\n \t * inflate() calls.\n \t */\n \n-\tif (sha1_object_info(base_sha1, type, NULL))\n+\tif (packed_object_info(&base_ent, type, NULL))\n \t\tdie(\"cannot get info for delta-pack base\");\n \n \tif (sizep) {\n@@ -716,7 +722,7 @@ static int packed_object_info(struct pac\n \n \tswitch (kind) {\n \tcase OBJ_DELTA:\n-\t\tretval = packed_delta_info(pack, size, left, type, sizep);\n+\t\tretval = packed_delta_info(pack, size, left, type, sizep, p);\n \t\tunuse_packed_git(p);\n \t\treturn retval;\n \tcase OBJ_COMMIT:\n@@ -747,8 +753,10 @@ static void *unpack_delta_entry(unsigned\n \t\t\t\tunsigned long delta_size,\n \t\t\t\tunsigned long left,\n \t\t\t\tchar *type,\n-\t\t\t\tunsigned long *sizep)\n+\t\t\t\tunsigned long *sizep,\n+\t\t\t\tstruct packed_git *p)\n {\n+\tstruct pack_entry base_ent;\n \tvoid *data, *delta_data, *result, *base;\n \tunsigned long data_size, result_size, base_size;\n \tz_stream stream;\n@@ -773,8 +781,11 @@ static void *unpack_delta_entry(unsigned\n \tif ((st != Z_STREAM_END) || stream.total_out != delta_size)\n \t\tdie(\"delta data unpack failed\");\n \n-\t/* This may recursively unpack the base, which is what we want */\n-\tbase = read_sha1_file(base_sha1, type, &base_size);\n+\t/* The base entry _must_ be in the same pack */\n+\tif (!find_pack_entry_one(base_sha1, &base_ent, p))\n+\t\tdie(\"failed to find delta-pack base object %s\",\n+\t\t    sha1_to_hex(base_sha1));\n+\tbase = unpack_entry_gently(&base_ent, type, &base_size);\n \tif (!base)\n \t\tdie(\"failed to read delta-pack base object %s\",\n \t\t    sha1_to_hex(base_sha1));\n@@ -820,21 +831,33 @@ static void *unpack_entry(struct pack_en\n \t\t\t  char *type, unsigned long *sizep)\n {\n \tstruct packed_git *p = entry->p;\n-\tunsigned long offset, size, left;\n-\tunsigned char *pack;\n-\tenum object_type kind;\n \tvoid *retval;\n \n \tif (use_packed_git(p))\n \t\tdie(\"cannot map packed file\");\n+\tretval = unpack_entry_gently(entry, type, sizep);\n+\tunuse_packed_git(p);\n+\tif (!retval)\n+\t\tdie(\"corrupted pack file\");\n+\treturn retval;\n+}\n+\n+/* The caller is responsible for use_packed_git()/unuse_packed_git() pair */\n+void *unpack_entry_gently(struct pack_entry *entry,\n+\t\t\t  char *type, unsigned long *sizep)\n+{\n+\tstruct packed_git *p = entry->p;\n+\tunsigned long offset, size, left;\n+\tunsigned char *pack;\n+\tenum object_type kind;\n+\tvoid *retval;\n \n \toffset = unpack_object_header(p, entry->offset, &kind, &size);\n \tpack = p->pack_base + offset;\n \tleft = p->pack_size - offset;\n \tswitch (kind) {\n \tcase OBJ_DELTA:\n-\t\tretval = unpack_delta_entry(pack, size, left, type, sizep);\n-\t\tunuse_packed_git(p);\n+\t\tretval = unpack_delta_entry(pack, size, left, type, sizep, p);\n \t\treturn retval;\n \tcase OBJ_COMMIT:\n \t\tstrcpy(type, \"commit\");\n@@ -849,11 +872,10 @@ static void *unpack_entry(struct pack_en\n \t\tstrcpy(type, \"tag\");\n \t\tbreak;\n \tdefault:\n-\t\tdie(\"corrupted pack file\");\n+\t\treturn NULL;\n \t}\n \t*sizep = size;\n \tretval = unpack_non_delta_entry(pack, size, left);\n-\tunuse_packed_git(p);\n \treturn retval;\n }\n \n@@ -873,8 +895,8 @@ int nth_packed_object_sha1(const struct \n \treturn 0;\n }\n \n-static int find_pack_entry_1(const unsigned char *sha1,\n-\t\t\t     struct pack_entry *e, struct packed_git *p)\n+int find_pack_entry_one(const unsigned char *sha1,\n+\t\t\tstruct pack_entry *e, struct packed_git *p)\n {\n \tint *level1_ofs = p->index_base;\n \tint hi = ntohl(level1_ofs[*sha1]);\n@@ -904,7 +926,7 @@ static int find_pack_entry(const unsigne\n \tprepare_packed_git();\n \n \tfor (p = packed_git; p; p = p->next) {\n-\t\tif (find_pack_entry_1(sha1, e, p))\n+\t\tif (find_pack_entry_one(sha1, e, p))\n \t\t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/verify-pack.c b/verify-pack.c\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -1,25 +1,56 @@\n #include \"cache.h\"\n #include \"pack.h\"\n \n-static int verify_one_pack(char *arg)\n+static int verify_one_pack(char *arg, int verbose)\n {\n-\tstruct packed_git *g = add_packed_git(arg, strlen(arg));\n-\tif (!g)\n-\t\treturn -1;\n-\treturn verify_pack(g);\n+\tint len = strlen(arg);\n+\tstruct packed_git *g;\n+\t\n+\twhile (1) {\n+\t\t/* Should name foo.idx, but foo.pack may be named;\n+\t\t * convert it to foo.idx\n+\t\t */\n+\t\tif (!strcmp(arg + len - 5, \".pack\")) {\n+\t\t\tstrcpy(arg + len - 5, \".idx\");\n+\t\t\tlen--;\n+\t\t}\n+\t\t/* Should name foo.idx now */\n+\t\tif ((g = add_packed_git(arg, len)))\n+\t\t\tbreak;\n+\t\t/* No?  did you name just foo? */\n+\t\tstrcpy(arg + len, \".idx\");\n+\t\tlen += 4;\n+\t\tif ((g = add_packed_git(arg, len)))\n+\t\t\tbreak;\n+\t\treturn error(\"packfile %s not found.\", arg);\n+\t}\n+\treturn verify_pack(g, verbose);\n }\n \n+static const char *verify_pack_usage = \"git-verify-pack [-v] <pack>...\";\n+\n int main(int ac, char **av)\n {\n \tint errs = 0;\n+\tint verbose = 0;\n+\tint no_more_options = 0;\n \n \twhile (1 < ac) {\n \t\tchar path[PATH_MAX];\n-\t\tstrcpy(path, av[1]);\n-\t\tif (verify_one_pack(path))\n-\t\t\terrs++;\n-\t\telse\n-\t\t\tprintf(\"%s: OK\\n\", av[1]);\n+\n+\t\tif (!no_more_options && av[1][0] == '-') {\n+\t\t\tif (!strcmp(\"-v\", av[1]))\n+\t\t\t\tverbose = 1;\n+\t\t\telse if (!strcmp(\"--\", av[1]))\n+\t\t\t\tno_more_options = 1;\n+\t\t\telse\n+\t\t\t\tusage(verify_pack_usage);\n+\t\t}\n+\t\telse {\n+\t\t\tstrcpy(path, av[1]);\n+\t\t\tif (verify_one_pack(path, verbose))\n+\t\t\t\terrs++;\n+\t\t}\n \t\tac--; av++;\n \t}\n \treturn !!errs;\n------------\n"}]}