{"thread":{"id":"7517","subject":"[PATCH] clean up and optimize nth_packed_object_sha1() usage","startedAt":"2007-04-04T20:49:04Z","lastAt":"2007-04-05T06:27:53Z","messageCount":2,"participants":["Nicolas Pitre","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"38653","messageId":"alpine.LFD.0.98.0704041639070.28181@xanadu.home","threadId":"7517","inReplyTo":null,"subject":"[PATCH] clean up and optimize nth_packed_object_sha1() usage","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-04T20:49:04Z","receivedAt":"2007-04-04T20:49:04Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Let's avoid the open coded pack index reference in pack-object and use \nnth_packed_object_sha1() instead.  This will help encapsulating index\nformat differences in one place.\n\nAnd while at it there is no reason to copy SHA1's over and over while a \ndirect pointer to it in the index will do just fine.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex 21f1f9e..4e5aa33 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -348,7 +348,7 @@ static int fsck_tag(struct tag *tag)\n \treturn 0;\n }\n \n-static int fsck_sha1(unsigned char *sha1)\n+static int fsck_sha1(const unsigned char *sha1)\n {\n \tstruct object *obj = parse_object(sha1);\n \tif (!obj) {\n@@ -648,11 +648,8 @@ int cmd_fsck(int argc, char **argv, const char *prefix)\n \n \t\tfor (p = packed_git; p; p = p->next) {\n \t\t\tuint32_t i, num = num_packed_objects(p);\n-\t\t\tfor (i = 0; i < num; i++) {\n-\t\t\t\tunsigned char sha1[20];\n-\t\t\t\tnth_packed_object_sha1(p, i, sha1);\n-\t\t\t\tfsck_sha1(sha1);\n-\t\t\t}\n+\t\t\tfor (i = 0; i < num; i++)\n+\t\t\t\tfsck_sha1(nth_packed_object_sha1(p, i));\n \t\t}\n \t}\n \ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex b5f9648..45ac3e4 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -222,7 +222,7 @@ static const unsigned char *find_packed_object_name(struct packed_git *p,\n \t\t\t\t\t\t    off_t ofs)\n {\n \tstruct revindex_entry *entry = find_packed_object(p, ofs);\n-\treturn ((unsigned char *)p->index_data) + 4 * 256 + 24 * entry->nr + 4;\n+\treturn nth_packed_object_sha1(p, entry->nr);\n }\n \n static void *delta_against(void *buf, unsigned long size, struct object_entry *entry)\ndiff --git a/cache.h b/cache.h\nindex 7199430..710554f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -435,7 +435,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t\n extern void unuse_pack(struct pack_window **);\n extern struct packed_git *add_packed_git(const char *, int, int);\n extern uint32_t num_packed_objects(const struct packed_git *p);\n-extern int nth_packed_object_sha1(const struct packed_git *, uint32_t, unsigned char*);\n+extern const unsigned char *nth_packed_object_sha1(const struct packed_git *, uint32_t);\n extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);\n extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);\n extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);\ndiff --git a/pack-check.c b/pack-check.c\nindex d988322..f58083d 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -42,13 +42,14 @@ static int verify_packfile(struct packed_git *p,\n \t */\n \tnr_objects = num_packed_objects(p);\n \tfor (i = 0, err = 0; i < nr_objects; i++) {\n-\t\tunsigned char sha1[20];\n+\t\tconst unsigned char *sha1;\n \t\tvoid *data;\n \t\tenum object_type type;\n \t\tunsigned long size;\n \t\toff_t offset;\n \n-\t\tif (nth_packed_object_sha1(p, i, sha1))\n+\t\tsha1 = nth_packed_object_sha1(p, i);\n+\t\tif (!sha1)\n \t\t\tdie(\"internal error pack-check nth-packed-object\");\n \t\toffset = find_pack_entry_one(sha1, p);\n \t\tif (!offset)\n@@ -82,14 +83,16 @@ static void show_pack_info(struct packed_git *p)\n \tmemset(chain_histogram, 0, sizeof(chain_histogram));\n \n \tfor (i = 0; i < nr_objects; i++) {\n-\t\tunsigned char sha1[20], base_sha1[20];\n+\t\tconst unsigned char *sha1;\n+\t\tunsigned char base_sha1[20];\n \t\tconst char *type;\n \t\tunsigned long size;\n \t\tunsigned long store_size;\n \t\toff_t offset;\n \t\tunsigned int delta_chain_length;\n \n-\t\tif (nth_packed_object_sha1(p, i, sha1))\n+\t\tsha1 = nth_packed_object_sha1(p, i);\n+\t\tif (!sha1)\n \t\t\tdie(\"internal error pack-check nth-packed-object\");\n \t\toffset = find_pack_entry_one(sha1, p);\n \t\tif (!offset)\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9c26038..4304fe9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1532,15 +1532,14 @@ uint32_t num_packed_objects(const struct packed_git *p)\n \treturn (uint32_t)((p->index_size - 20 - 20 - 4*256) / 24);\n }\n \n-int nth_packed_object_sha1(const struct packed_git *p, uint32_t n,\n-\t\t\t   unsigned char* sha1)\n+const unsigned char *nth_packed_object_sha1(const struct packed_git *p,\n+\t\t\t\t\t    uint32_t n)\n {\n \tconst unsigned char *index = p->index_data;\n \tindex += 4 * 256;\n \tif (num_packed_objects(p) <= n)\n-\t\treturn -1;\n-\thashcpy(sha1, index + 24 * n + 4);\n-\treturn 0;\n+\t\treturn NULL;\n+\treturn index + 24 * n + 4;\n }\n \n off_t find_pack_entry_one(const unsigned char *sha1,\ndiff --git a/sha1_name.c b/sha1_name.c\nindex bede0e5..267ea3f 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -71,7 +71,7 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *\n static int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1)\n {\n \tstruct packed_git *p;\n-\tunsigned char found_sha1[20];\n+\tconst unsigned char *found_sha1 = NULL;\n \tint found = 0;\n \n \tprepare_packed_git();\n@@ -80,10 +80,10 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \t\tuint32_t first = 0, last = num;\n \t\twhile (first < last) {\n \t\t\tuint32_t mid = (first + last) / 2;\n-\t\t\tunsigned char now[20];\n+\t\t\tconst unsigned char *now;\n \t\t\tint cmp;\n \n-\t\t\tnth_packed_object_sha1(p, mid, now);\n+\t\t\tnow = nth_packed_object_sha1(p, mid);\n \t\t\tcmp = hashcmp(match, now);\n \t\t\tif (!cmp) {\n \t\t\t\tfirst = mid;\n@@ -96,14 +96,14 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \t\t\tlast = mid;\n \t\t}\n \t\tif (first < num) {\n-\t\t\tunsigned char now[20], next[20];\n-\t\t\tnth_packed_object_sha1(p, first, now);\n+\t\t\tconst unsigned char *now, *next;\n+\t\t       now = nth_packed_object_sha1(p, first);\n \t\t\tif (match_sha(len, match, now)) {\n-\t\t\t\tif (nth_packed_object_sha1(p, first+1, next) ||\n-\t\t\t\t    !match_sha(len, match, next)) {\n+\t\t\t\tnext = nth_packed_object_sha1(p, first+1);\n+\t\t\t       if (!next|| !match_sha(len, match, next)) {\n \t\t\t\t\t/* unique within this pack */\n \t\t\t\t\tif (!found) {\n-\t\t\t\t\t\thashcpy(found_sha1, now);\n+\t\t\t\t\t\tfound_sha1 = now;\n \t\t\t\t\t\tfound++;\n \t\t\t\t\t}\n \t\t\t\t\telse if (hashcmp(found_sha1, now)) {\n"},{"id":"38677","messageId":"20070405062753.GC5436@spearce.org","threadId":"7517","inReplyTo":"alpine.LFD.0.98.0704041639070.28181@xanadu.home","subject":"Re: [PATCH] clean up and optimize nth_packed_object_sha1() usage","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-05T06:27:53Z","receivedAt":"2007-04-05T06:27:53Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> Let's avoid the open coded pack index reference in pack-object and use \n> nth_packed_object_sha1() instead.  This will help encapsulating index\n> format differences in one place.\n> \n> And while at it there is no reason to copy SHA1's over and over while a \n> direct pointer to it in the index will do just fine.\n\nAck++.  ;-)\n\nThis is a good change.  That API has been bothering me for a while.\n \n> @@ -96,14 +96,14 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n>  \t\t\tlast = mid;\n>  \t\t}\n>  \t\tif (first < num) {\n> -\t\t\tunsigned char now[20], next[20];\n> -\t\t\tnth_packed_object_sha1(p, first, now);\n> +\t\t\tconst unsigned char *now, *next;\n> +\t\t       now = nth_packed_object_sha1(p, first);\n>  \t\t\tif (match_sha(len, match, now)) {\n\nUh, there is some sort of werid whitespace indent damage on that\nsecond added line.  It doesn't line up with the first added line,\nor with either of the context lines.\n\n-- \nShawn.\n"}]}