{"thread":{"id":"7275","subject":"[PATCH] clean up pack index handling a bit","startedAt":"2007-03-16T20:42:50Z","lastAt":"2007-03-17T05:04:50Z","messageCount":2,"participants":["Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37276","messageId":"alpine.LFD.0.83.0703161639270.18328@xanadu.home","threadId":"7275","inReplyTo":null,"subject":"[PATCH] clean up pack index handling a bit","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-03-16T20:42:50Z","receivedAt":"2007-03-16T20:42:50Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"\nEspecially with the new index format to come, it is more appropriate\nto encapsulate more into check_packed_git_idx() and assume less of the\nindex format in struct packed_git.\n\nTo that effect, the index_base is renamed to index_data with void * type\nso it is not used directly but other pointers initialized with it. This\nallows for a couple pointer cast removal, as well as providing a better\ngeneric name to grep for when adding support for new index versions or\nformats.\n\nAnd index_data is declared const too while at it.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex f8ebad0..73d448b 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -166,11 +166,12 @@ static void prepare_pack_revindex(struct pack_revindex *rix)\n \tstruct packed_git *p = rix->p;\n \tint num_ent = num_packed_objects(p);\n \tint i;\n-\tvoid *index = p->index_base + 256;\n+\tconst char *index = p->index_data;\n \n+\tindex += 4 * 256;\n \trix->revindex = xmalloc(sizeof(*rix->revindex) * (num_ent + 1));\n \tfor (i = 0; i < num_ent; i++) {\n-\t\tunsigned int hl = *((unsigned int *)((char *) index + 24*i));\n+\t\tuint32_t hl = *((uint32_t *)(index + 24 * i));\n \t\trix->revindex[i].offset = ntohl(hl);\n \t\trix->revindex[i].nr = i;\n \t}\n@@ -217,11 +218,11 @@ static off_t find_packed_object_size(struct packed_git *p, off_t ofs)\n \treturn entry[1].offset - ofs;\n }\n \n-static unsigned char *find_packed_object_name(struct packed_git *p,\n-\t\t\t\t\t      off_t ofs)\n+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_base + 256) + 24 * entry->nr + 4;\n+\treturn ((unsigned char *)p->index_data) + 4 * 256 + 24 * entry->nr + 4;\n }\n \n static void *delta_against(void *buf, unsigned long size, struct object_entry *entry)\n@@ -996,7 +997,8 @@ static void check_object(struct object_entry *entry)\n \t\t * delta.\n \t\t */\n \t\tif (!no_reuse_delta) {\n-\t\t\tunsigned char c, *base_name;\n+\t\t\tunsigned char c;\n+\t\t\tconst unsigned char *base_name;\n \t\t\toff_t ofs;\n \t\t\tunsigned long used_0;\n \t\t\t/* there is at least 20 bytes left in the pack */\ndiff --git a/cache.h b/cache.h\nindex 9f8e655..5396d33 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -371,10 +371,11 @@ struct pack_window {\n extern struct packed_git {\n \tstruct packed_git *next;\n \tstruct pack_window *windows;\n-\tuint32_t *index_base;\n-\ttime_t mtime;\n+\tconst void *index_data;\n \toff_t index_size;\n \toff_t pack_size;\n+\ttime_t mtime;\n+\tint index_version;\n \tint pack_fd;\n \tint pack_local;\n \tunsigned char sha1[20];\n@@ -412,7 +413,7 @@ extern int server_supports(const char *feature);\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1);\n extern struct packed_git *parse_pack_index_file(const unsigned char *sha1,\n-\t\t\t\t\t\tchar *idx_path);\n+\t\t\t\t\t\tconst char *idx_path);\n \n extern void prepare_packed_git(void);\n extern void reprepare_packed_git(void);\n@@ -424,7 +425,7 @@ extern struct packed_git *find_sha1_pack(const unsigned char *sha1,\n extern void pack_report(void);\n extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t, unsigned int *);\n extern void unuse_pack(struct pack_window **);\n-extern struct packed_git *add_packed_git(char *, int, int);\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 off_t find_pack_entry_one(const unsigned char *, struct packed_git *);\ndiff --git a/pack-check.c b/pack-check.c\nindex 299c514..d988322 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -5,7 +5,7 @@ static int verify_packfile(struct packed_git *p,\n \t\tstruct pack_window **w_curs)\n {\n \toff_t index_size = p->index_size;\n-\tvoid *index_base = p->index_base;\n+\tconst unsigned char *index_base = p->index_data;\n \tSHA_CTX ctx;\n \tunsigned char sha1[20];\n \toff_t offset = 0, pack_sig = p->pack_size - 20;\n@@ -31,7 +31,7 @@ static int verify_packfile(struct packed_git *p,\n \tif (hashcmp(sha1, use_pack(p, w_curs, pack_sig, NULL)))\n \t\treturn error(\"Packfile %s SHA1 mismatch with itself\",\n \t\t\t     p->pack_name);\n-\tif (hashcmp(sha1, (unsigned char *)index_base + index_size - 40))\n+\tif (hashcmp(sha1, index_base + index_size - 40))\n \t\treturn error(\"Packfile %s SHA1 mismatch with idx\",\n \t\t\t     p->pack_name);\n \tunuse_pack(w_curs);\n@@ -127,7 +127,7 @@ static void show_pack_info(struct packed_git *p)\n int verify_pack(struct packed_git *p, int verbose)\n {\n \toff_t index_size = p->index_size;\n-\tvoid *index_base = p->index_base;\n+\tconst unsigned char *index_base = p->index_data;\n \tSHA_CTX ctx;\n \tunsigned char sha1[20];\n \tint ret;\n@@ -137,7 +137,7 @@ int verify_pack(struct packed_git *p, int verbose)\n \tSHA1_Init(&ctx);\n \tSHA1_Update(&ctx, index_base, (unsigned int)(index_size - 20));\n \tSHA1_Final(sha1, &ctx);\n-\tif (hashcmp(sha1, (unsigned char *)index_base + index_size - 20))\n+\tif (hashcmp(sha1, index_base + index_size - 20))\n \t\tret = error(\"Packfile index for %s SHA1 mismatch\",\n \t\t\t    p->pack_name);\n \ndiff --git a/pack-redundant.c b/pack-redundant.c\nindex c8f7d9a..40e579b 100644\n--- a/pack-redundant.c\n+++ b/pack-redundant.c\n@@ -17,7 +17,7 @@ static int load_all_packs, verbose, alt_odb;\n \n struct llist_item {\n \tstruct llist_item *next;\n-\tunsigned char *sha1;\n+\tconst unsigned char *sha1;\n };\n static struct llist {\n \tstruct llist_item *front;\n@@ -104,9 +104,9 @@ static struct llist * llist_copy(struct llist *list)\n \treturn ret;\n }\n \n-static inline struct llist_item * llist_insert(struct llist *list,\n-\t\t\t\t\t       struct llist_item *after,\n-\t\t\t\t\t       unsigned char *sha1)\n+static inline struct llist_item *llist_insert(struct llist *list,\n+\t\t\t\t\t      struct llist_item *after,\n+\t\t\t\t\t       const unsigned char *sha1)\n {\n \tstruct llist_item *new = llist_item_get();\n \tnew->sha1 = sha1;\n@@ -128,12 +128,14 @@ static inline struct llist_item * llist_insert(struct llist *list,\n \treturn new;\n }\n \n-static inline struct llist_item *llist_insert_back(struct llist *list, unsigned char *sha1)\n+static inline struct llist_item *llist_insert_back(struct llist *list,\n+\t\t\t\t\t\t   const unsigned char *sha1)\n {\n \treturn llist_insert(list, list->back, sha1);\n }\n \n-static inline struct llist_item *llist_insert_sorted_unique(struct llist *list, unsigned char *sha1, struct llist_item *hint)\n+static inline struct llist_item *llist_insert_sorted_unique(struct llist *list,\n+\t\t\tconst unsigned char *sha1, struct llist_item *hint)\n {\n \tstruct llist_item *prev = NULL, *l;\n \n@@ -246,12 +248,12 @@ static struct pack_list * pack_list_difference(const struct pack_list *A,\n static void cmp_two_packs(struct pack_list *p1, struct pack_list *p2)\n {\n \tint p1_off, p2_off;\n-\tunsigned char *p1_base, *p2_base;\n+\tconst unsigned char *p1_base, *p2_base;\n \tstruct llist_item *p1_hint = NULL, *p2_hint = NULL;\n \n \tp1_off = p2_off = 256 * 4 + 4;\n-\tp1_base = (unsigned char *) p1->pack->index_base;\n-\tp2_base = (unsigned char *) p2->pack->index_base;\n+\tp1_base = p1->pack->index_data;\n+\tp2_base = p2->pack->index_data;\n \n \twhile (p1_off <= p1->pack->index_size - 3 * 20 &&\n \t       p2_off <= p2->pack->index_size - 3 * 20)\n@@ -351,11 +353,11 @@ static size_t sizeof_union(struct packed_git *p1, struct packed_git *p2)\n {\n \tsize_t ret = 0;\n \tint p1_off, p2_off;\n-\tunsigned char *p1_base, *p2_base;\n+\tconst unsigned char *p1_base, *p2_base;\n \n \tp1_off = p2_off = 256 * 4 + 4;\n-\tp1_base = (unsigned char *)p1->index_base;\n-\tp2_base = (unsigned char *)p2->index_base;\n+\tp1_base = p1->index_data;\n+\tp2_base = p2->index_data;\n \n \twhile (p1_off <= p1->index_size - 3 * 20 &&\n \t       p2_off <= p2->index_size - 3 * 20)\n@@ -534,7 +536,7 @@ static struct pack_list * add_pack(struct packed_git *p)\n {\n \tstruct pack_list l;\n \tsize_t off;\n-\tunsigned char *base;\n+\tconst unsigned char *base;\n \n \tif (!p->pack_local && !(alt_odb || verbose))\n \t\treturn NULL;\n@@ -543,7 +545,7 @@ static struct pack_list * add_pack(struct packed_git *p)\n \tllist_init(&l.all_objects);\n \n \toff = 256 * 4 + 4;\n-\tbase = (unsigned char *)p->index_base;\n+\tbase = p->index_data;\n \twhile (off <= p->index_size - 3 * 20) {\n \t\tllist_insert_back(l.all_objects, base + off);\n \t\toff += 24;\ndiff --git a/pack.h b/pack.h\nindex deb427e..d4d412c 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -16,24 +16,15 @@ struct pack_header {\n };\n \n /*\n- * Packed object index header\n- *\n- * struct pack_idx_header {\n- * \tuint32_t idx_signature;\n- *\tuint32_t idx_version;\n- * };\n- *\n- * Note: this header isn't active yet.  In future versions of git\n- * we may change the index file format.  At that time we would start\n- * the first four bytes of the new index format with this signature,\n- * as all older git binaries would find this value illegal and abort\n- * reading the file.\n+ * The first four bytes of index formats later than version 1 should\n+ * start with this signature, as all older git binaries would find this\n+ * value illegal and abort reading the file.\n  *\n  * This is the case because the number of objects in a packfile\n  * cannot exceed 1,431,660,000 as every object would need at least\n- * 3 bytes of data and the overall packfile cannot exceed 4 GiB due\n- * to the 32 bit offsets used by the index.  Clearly the signature\n- * exceeds this maximum.\n+ * 3 bytes of data and the overall packfile cannot exceed 4 GiB with\n+ * version 1 of the index file due to the offsets limited to 32 bits.\n+ * Clearly the signature exceeds this maximum.\n  *\n  * Very old git binaries will also compare the first 4 bytes to the\n  * next 4 bytes in the index and abort with a \"non-monotonic index\"\n@@ -43,6 +34,15 @@ struct pack_header {\n  */\n #define PACK_IDX_SIGNATURE 0xff744f63\t/* \"\\377tOc\" */\n \n+/*\n+ * Packed object index header\n+ */\n+struct pack_idx_header {\n+\tuint32_t idx_signature;\n+\tuint32_t idx_version;\n+};\n+\n+\n extern int verify_pack(struct packed_git *, int);\n \n #define PH_ERROR_EOF\t\t(-1)\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 5691448..110d696 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -432,16 +432,15 @@ void pack_report()\n \t\tpack_mapped, peak_pack_mapped);\n }\n \n-static int check_packed_git_idx(const char *path,\n-\tunsigned long *idx_size_,\n-\tvoid **idx_map_)\n+static int check_packed_git_idx(const char *path,  struct packed_git *p)\n {\n \tvoid *idx_map;\n-\tuint32_t *index;\n+\tstruct pack_idx_header *hdr;\n \tsize_t idx_size;\n-\tuint32_t nr, i;\n+\tuint32_t nr, i, *index;\n \tint fd = open(path, O_RDONLY);\n \tstruct stat st;\n+\n \tif (fd < 0)\n \t\treturn -1;\n \tif (fstat(fd, &st)) {\n@@ -456,15 +455,12 @@ static int check_packed_git_idx(const char *path,\n \tidx_map = xmmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \n-\tindex = idx_map;\n-\t*idx_map_ = idx_map;\n-\t*idx_size_ = idx_size;\n-\n \t/* a future index format would start with this, as older git\n \t * binaries would fail the non-monotonic index check below.\n \t * give a nicer warning to the user if we can.\n \t */\n-\tif (index[0] == htonl(PACK_IDX_SIGNATURE)) {\n+\thdr = idx_map;\n+\tif (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {\n \t\tmunmap(idx_map, idx_size);\n \t\treturn error(\"index file %s is a newer version\"\n \t\t\t\" and is not supported by this binary\"\n@@ -473,6 +469,7 @@ static int check_packed_git_idx(const char *path,\n \t}\n \n \tnr = 0;\n+\tindex = idx_map;\n \tfor (i = 0; i < 256; i++) {\n \t\tuint32_t n = ntohl(index[i]);\n \t\tif (n < nr) {\n@@ -494,6 +491,9 @@ static int check_packed_git_idx(const char *path,\n \t\treturn error(\"wrong index file size in %s\", path);\n \t}\n \n+\tp->index_version = 1;\n+\tp->index_data = idx_map;\n+\tp->index_size = idx_size;\n \treturn 0;\n }\n \n@@ -614,7 +614,7 @@ static int open_packed_git_1(struct packed_git *p)\n \t\treturn error(\"end of packfile %s is unavailable\", p->pack_name);\n \tif (read_in_full(p->pack_fd, sha1, sizeof(sha1)) != sizeof(sha1))\n \t\treturn error(\"packfile %s signature is unavailable\", p->pack_name);\n-\tidx_sha1 = ((unsigned char *)p->index_base) + p->index_size - 40;\n+\tidx_sha1 = ((unsigned char *)p->index_data) + p->index_size - 40;\n \tif (hashcmp(sha1, idx_sha1))\n \t\treturn error(\"packfile %s does not match index\", p->pack_name);\n \treturn 0;\n@@ -710,38 +710,37 @@ unsigned char* use_pack(struct packed_git *p,\n \treturn win->base + offset;\n }\n \n-struct packed_git *add_packed_git(char *path, int path_len, int local)\n+struct packed_git *add_packed_git(const char *path, int path_len, int local)\n {\n \tstruct stat st;\n-\tstruct packed_git *p;\n-\tunsigned long idx_size;\n-\tvoid *idx_map;\n-\tunsigned char sha1[20];\n+\tstruct packed_git *p = xmalloc(sizeof(*p) + path_len + 2);\n \n-\tif (check_packed_git_idx(path, &idx_size, &idx_map))\n+\t/*\n+\t * Make sure a corresponding .pack file exists and that\n+\t * the index looks sane.\n+\t */\n+\tpath_len -= strlen(\".idx\");\n+\tif (path_len < 1)\n \t\treturn NULL;\n-\n-\t/* do we have a corresponding .pack file? */\n-\tstrcpy(path + path_len - 4, \".pack\");\n-\tif (stat(path, &st) || !S_ISREG(st.st_mode)) {\n-\t\tmunmap(idx_map, idx_size);\n+\tmemcpy(p->pack_name, path, path_len);\n+\tstrcpy(p->pack_name + path_len, \".pack\");\n+\tif (stat(p->pack_name, &st) || !S_ISREG(st.st_mode) ||\n+\t    check_packed_git_idx(path, p)) {\n+\t\tfree(p);\n \t\treturn NULL;\n \t}\n+\n \t/* ok, it looks sane as far as we can check without\n \t * actually mapping the pack file.\n \t */\n-\tp = xmalloc(sizeof(*p) + path_len + 2);\n-\tstrcpy(p->pack_name, path);\n-\tp->index_size = idx_size;\n \tp->pack_size = st.st_size;\n-\tp->index_base = idx_map;\n \tp->next = NULL;\n \tp->windows = NULL;\n \tp->pack_fd = -1;\n \tp->pack_local = local;\n \tp->mtime = st.st_mtime;\n-\tif ((path_len > 44) && !get_sha1_hex(path + path_len - 44, sha1))\n-\t\thashcpy(p->sha1, sha1);\n+\tif (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))\n+\t\thashclr(p->sha1);\n \treturn p;\n }\n \n@@ -751,23 +750,19 @@ struct packed_git *parse_pack_index(unsigned char *sha1)\n \treturn parse_pack_index_file(sha1, path);\n }\n \n-struct packed_git *parse_pack_index_file(const unsigned char *sha1, char *idx_path)\n+struct packed_git *parse_pack_index_file(const unsigned char *sha1,\n+\t\t\t\t\t const char *idx_path)\n {\n-\tstruct packed_git *p;\n-\tunsigned long idx_size;\n-\tvoid *idx_map;\n-\tchar *path;\n+\tconst char *path = sha1_pack_name(sha1);\n+\tstruct packed_git *p = xmalloc(sizeof(*p) + strlen(path) + 2);\n \n-\tif (check_packed_git_idx(idx_path, &idx_size, &idx_map))\n+\tif (check_packed_git_idx(idx_path, p)) {\n+\t\tfree(p);\n \t\treturn NULL;\n+\t}\n \n-\tpath = sha1_pack_name(sha1);\n-\n-\tp = xmalloc(sizeof(*p) + strlen(path) + 2);\n \tstrcpy(p->pack_name, path);\n-\tp->index_size = idx_size;\n \tp->pack_size = 0;\n-\tp->index_base = idx_map;\n \tp->next = NULL;\n \tp->windows = NULL;\n \tp->pack_fd = -1;\n@@ -1423,24 +1418,27 @@ uint32_t num_packed_objects(const struct packed_git *p)\n int nth_packed_object_sha1(const struct packed_git *p, uint32_t n,\n \t\t\t   unsigned char* sha1)\n {\n-\tvoid *index = p->index_base + 256;\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, (unsigned char *) index + (24 * n) + 4);\n+\thashcpy(sha1, index + 24 * n + 4);\n \treturn 0;\n }\n \n off_t find_pack_entry_one(const unsigned char *sha1,\n \t\t\t\t  struct packed_git *p)\n {\n-\tuint32_t *level1_ofs = p->index_base;\n+\tconst uint32_t *level1_ofs = p->index_data;\n \tint hi = ntohl(level1_ofs[*sha1]);\n \tint lo = ((*sha1 == 0x0) ? 0 : ntohl(level1_ofs[*sha1 - 1]));\n-\tvoid *index = p->index_base + 256;\n+\tconst unsigned char *index = p->index_data;\n+\n+\tindex += 4 * 256;\n \n \tdo {\n \t\tint mi = (lo + hi) / 2;\n-\t\tint cmp = hashcmp((unsigned char *)index + (24 * mi) + 4, sha1);\n+\t\tint cmp = hashcmp(index + 24 * mi + 4, sha1);\n \t\tif (!cmp)\n \t\t\treturn ntohl(*((uint32_t *)((char *)index + (24 * mi))));\n \t\tif (cmp > 0)\n"},{"id":"37301","messageId":"7vlkhwtq8d.fsf@assigned-by-dhcp.cox.net","threadId":"7275","inReplyTo":"alpine.LFD.0.83.0703161639270.18328@xanadu.home","subject":"Re: [PATCH] clean up pack index handling a bit","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-17T05:04:50Z","receivedAt":"2007-03-17T05:04:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}