git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 08/41] packfile: abstract away hash constant values

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Apr 23, 2018, 23:39 UTC
Message-ID
<20180423233951.276447-9-sandals@crustytoothpaste.net>
In-Reply-To
<20180423233951.276447-1-sandals@crustytoothpaste.net>

There are several instances of the constant 20 and 20-based values in the packfile code. Abstract away dependence on SHA-1 by using the values from the_hash_algo instead.

Use unsigned values for temporary constants to provide the compiler with more information about what kinds of values it should expect.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 packfile.c | 66 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 29 deletions(-)
diff --git a/packfile.c b/packfile.c
index 84acd405e0..b7bc4eab17 100644
--- a/packfile.c
+++ b/packfile.c
@@ -84,6 +84,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 	uint32_t version, nr, i, *index;
 	int fd = git_open(path);
 	struct stat st;
+	const unsigned int hashsz = the_hash_algo->rawsz;
 
 	if (fd < 0)
 		return -1;
@@ -92,7 +93,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		return -1;
 	}
 	idx_size = xsize_t(st.st_size);
-	if (idx_size < 4 * 256 + 20 + 20) {
+	if (idx_size < 4 * 256 + hashsz + hashsz) {
 		close(fd);
 		return error("index file %s is too small", path);
 	}
@@ -129,11 +130,11 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		/*
 		 * Total size:
 		 *  - 256 index entries 4 bytes each
-		 *  - 24-byte entries * nr (20-byte sha1 + 4-byte offset)
-		 *  - 20-byte SHA1 of the packfile
-		 *  - 20-byte SHA1 file checksum
+		 *  - 24-byte entries * nr (object ID + 4-byte offset)
+		 *  - hash of the packfile
+		 *  - file checksum
 		 */
-		if (idx_size != 4*256 + nr * 24 + 20 + 20) {
+		if (idx_size != 4*256 + nr * (hashsz + 4) + hashsz + hashsz) {
 			munmap(idx_map, idx_size);
 			return error("wrong index v1 file size in %s", path);
 		}
@@ -142,16 +143,16 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 		 * Minimum size:
 		 *  - 8 bytes of header
 		 *  - 256 index entries 4 bytes each
-		 *  - 20-byte sha1 entry * nr
+		 *  - object ID entry * nr
 		 *  - 4-byte crc entry * nr
 		 *  - 4-byte offset entry * nr
-		 *  - 20-byte SHA1 of the packfile
-		 *  - 20-byte SHA1 file checksum
+		 *  - hash of the packfile
+		 *  - file checksum
 		 * And after the 4-byte offset table might be a
 		 * variable sized table containing 8-byte entries
 		 * for offsets larger than 2^31.
 		 */
-		unsigned long min_size = 8 + 4*256 + nr*(20 + 4 + 4) + 20 + 20;
+		unsigned long min_size = 8 + 4*256 + nr*(hashsz + 4 + 4) + hashsz + hashsz;
 		unsigned long max_size = min_size;
 		if (nr)
 			max_size += (nr - 1)*8;
@@ -444,10 +445,11 @@ static int open_packed_git_1(struct packed_git *p)
 {
 	struct stat st;
 	struct pack_header hdr;
-	unsigned char sha1[20];
-	unsigned char *idx_sha1;
+	unsigned char hash[GIT_MAX_RAWSZ];
+	unsigned char *idx_hash;
 	long fd_flag;
 	ssize_t read_result;
+	const unsigned hashsz = the_hash_algo->rawsz;
 
 	if (!p->index_data && open_pack_index(p))
 		return error("packfile %s index unavailable", p->pack_name);
@@ -507,15 +509,15 @@ static int open_packed_git_1(struct packed_git *p)
 			     " while index indicates %"PRIu32" objects",
 			     p->pack_name, ntohl(hdr.hdr_entries),
 			     p->num_objects);
-	if (lseek(p->pack_fd, p->pack_size - sizeof(sha1), SEEK_SET) == -1)
+	if (lseek(p->pack_fd, p->pack_size - hashsz, SEEK_SET) == -1)
 		return error("end of packfile %s is unavailable", p->pack_name);
-	read_result = read_in_full(p->pack_fd, sha1, sizeof(sha1));
+	read_result = read_in_full(p->pack_fd, hash, hashsz);
 	if (read_result < 0)
 		return error_errno("error reading from %s", p->pack_name);
-	if (read_result != sizeof(sha1))
+	if (read_result != hashsz)
 		return error("packfile %s signature is unavailable", p->pack_name);
-	idx_sha1 = ((unsigned char *)p->index_data) + p->index_size - 40;
-	if (hashcmp(sha1, idx_sha1))
+	idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2;
+	if (hashcmp(hash, idx_hash))
 		return error("packfile %s does not match index", p->pack_name);
 	return 0;
 }
@@ -530,7 +532,7 @@ static int open_packed_git(struct packed_git *p)
 
 static int in_window(struct pack_window *win, off_t offset)
 {
-	/* We must promise at least 20 bytes (one hash) after the
+	/* We must promise at least one full hash after the
 	 * offset is available from this window, otherwise the offset
 	 * is not actually in this window and a different window (which
 	 * has that one hash excess) must be used.  This is to support
@@ -538,7 +540,7 @@ static int in_window(struct pack_window *win, off_t offset)
 	 */
 	off_t win_off = win->offset;
 	return win_off <= offset
-		&& (offset + 20) <= (win_off + win->len);
+		&& (offset + the_hash_algo->rawsz) <= (win_off + win->len);
 }
 
 unsigned char *use_pack(struct packed_git *p,
@@ -555,7 +557,7 @@ unsigned char *use_pack(struct packed_git *p,
 	 */
 	if (!p->pack_size && p->pack_fd == -1 && open_packed_git(p))
 		die("packfile %s cannot be accessed", p->pack_name);
-	if (offset > (p->pack_size - 20))
+	if (offset > (p->pack_size - the_hash_algo->rawsz))
 		die("offset beyond end of packfile (truncated pack?)");
 	if (offset < 0)
 		die(_("offset before end of packfile (broken .idx?)"));
@@ -675,7 +677,8 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
 	p->pack_size = st.st_size;
 	p->pack_local = local;
 	p->mtime = st.st_mtime;
-	if (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))
+	if (path_len < the_hash_algo->hexsz ||
+	    get_sha1_hex(path + path_len - the_hash_algo->hexsz, p->sha1))
 		hashclr(p->sha1);
 	return p;
 }
@@ -1028,7 +1031,8 @@ const struct packed_git *has_packed_and_bad(const unsigned char *sha1)
 
 	for (p = the_repository->objects->packed_git; p; p = p->next)
 		for (i = 0; i < p->num_bad_objects; i++)
-			if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
+			if (!hashcmp(sha1,
+				     p->bad_object_sha1 + the_hash_algo->rawsz * i))
 				return p;
 	return NULL;
 }
@@ -1066,7 +1070,7 @@ static off_t get_delta_base(struct packed_git *p,
 	} else if (type == OBJ_REF_DELTA) {
 		/* The base entry _must_ be in the same pack */
 		base_offset = find_pack_entry_one(base_info, p);
-		*curpos += 20;
+		*curpos += the_hash_algo->rawsz;
 	} else
 		die("I am totally screwed");
 	return base_offset;
@@ -1671,6 +1675,7 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32
 {
 	const unsigned char *index_fanout = p->index_data;
 	const unsigned char *index_lookup;
+	const unsigned int hashsz = the_hash_algo->rawsz;
 	int index_lookup_width;
 
 	if (!index_fanout)
@@ -1678,10 +1683,10 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32
 
 	index_lookup = index_fanout + 4 * 256;
 	if (p->index_version == 1) {
-		index_lookup_width = 24;
+		index_lookup_width = hashsz + 4;
 		index_lookup += 4;
 	} else {
-		index_lookup_width = 20;
+		index_lookup_width = hashsz;
 		index_fanout += 8;
 		index_lookup += 8;
 	}
@@ -1694,6 +1699,7 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 					    uint32_t n)
 {
 	const unsigned char *index = p->index_data;
+	const unsigned int hashsz = the_hash_algo->rawsz;
 	if (!index) {
 		if (open_pack_index(p))
 			return NULL;
@@ -1703,10 +1709,10 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 		return NULL;
 	index += 4 * 256;
 	if (p->index_version == 1) {
-		return index + 24 * n + 4;
+		return index + (hashsz + 4) * n + 4;
 	} else {
 		index += 8;
-		return index + 20 * n;
+		return index + hashsz * n;
 	}
 }
 
@@ -1738,12 +1744,13 @@ void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
 {
 	const unsigned char *index = p->index_data;
+	const unsigned int hashsz = the_hash_algo->rawsz;
 	index += 4 * 256;
 	if (p->index_version == 1) {
-		return ntohl(*((uint32_t *)(index + 24 * n)));
+		return ntohl(*((uint32_t *)(index + (hashsz + 4) * n)));
 	} else {
 		uint32_t off;
-		index += 8 + p->num_objects * (20 + 4);
+		index += 8 + p->num_objects * (hashsz + 4);
 		off = ntohl(*((uint32_t *)(index + 4 * n)));
 		if (!(off & 0x80000000))
 			return off;
@@ -1814,7 +1821,8 @@ static int fill_pack_entry(const struct object_id *oid,
 	if (p->num_bad_objects) {
 		unsigned i;
 		for (i = 0; i < p->num_bad_objects; i++)
-			if (!hashcmp(oid->hash, p->bad_object_sha1 + 20 * i))
+			if (!hashcmp(oid->hash,
+				     p->bad_object_sha1 + the_hash_algo->rawsz * i))
 				return 0;
 	}
Previous: brian m. carlsonNext: Duy Nguyen
Message 69 of 76 in “object_id part 13”
  1. 00/41 object_id part 13brian m. carlson, Apr 23, 2018
  2. 01/41 cache: add a function to read an object ID from a bufferbrian m. carlson, Apr 23, 2018
  3. Martin ÅgrenApr 24, 2018
  4. Duy NguyenMay 1, 2018
  5. brian m. carlsonMay 1, 2018
  6. 02/41 server-info: remove unused members from struct pack_infobrian m. carlson, Apr 23, 2018
  7. Martin ÅgrenApr 24, 2018
  8. Duy NguyenMay 1, 2018
  9. 03/41 Remove unused member in struct object_contextbrian m. carlson, Apr 23, 2018
  10. Duy NguyenMay 1, 2018
  11. 04/41 packfile: remove unused member from struct pack_entrybrian m. carlson, Apr 23, 2018
  12. Duy NguyenMay 1, 2018
  13. 06/41 sha1_file: convert freshen functions to object_idbrian m. carlson, Apr 23, 2018
  14. 07/41 packfile: convert find_pack_entry to object_idbrian m. carlson, Apr 23, 2018
  15. 09/41 pack-objects: abstract away hash algorithmbrian m. carlson, Apr 23, 2018
  16. Duy NguyenMay 1, 2018
  17. 17/41 pack-redundant: convert linked lists to use struct object_idbrian m. carlson, Apr 23, 2018
  18. 16/41 Update struct index_state to use struct object_idbrian m. carlson, Apr 23, 2018
  19. 22/41 revision: replace use of hard-coded constantsbrian m. carlson, Apr 23, 2018
  20. 21/41 http: eliminate hard-coded constantsbrian m. carlson, Apr 23, 2018
  21. Martin ÅgrenApr 24, 2018
  22. Junio C HamanoApr 24, 2018
  23. brian m. carlsonApr 25, 2018
  24. 20/41 dir: convert struct untracked_cache_dir to object_idbrian m. carlson, Apr 23, 2018
  25. 25/41 builtin/receive-pack: avoid hard-coded constants for push certsbrian m. carlson, Apr 23, 2018
  26. Martin ÅgrenApr 24, 2018
  27. brian m. carlsonApr 25, 2018
  28. Martin ÅgrenApr 25, 2018
  29. 28/41 merge: convert empty tree constant to the_hash_algobrian m. carlson, Apr 23, 2018
  30. 36/41 sequencer: use the_hash_algo for empty tree object IDbrian m. carlson, Apr 23, 2018
  31. 34/41 sha1_file: convert cached object code to struct object_idbrian m. carlson, Apr 23, 2018
  32. 30/41 submodule: convert several uses of EMPTY_TREE_SHA1_HEXbrian m. carlson, Apr 23, 2018
  33. 40/41 add--interactive: compute the empty tree valuebrian m. carlson, Apr 23, 2018
  34. 39/41 Update shell scripts to compute empty tree object IDbrian m. carlson, Apr 23, 2018
  35. Duy NguyenMay 1, 2018
  36. brian m. carlsonMay 4, 2018
  37. 38/41 sha1_file: only expose empty object constants through git_hash_algobrian m. carlson, Apr 23, 2018
  38. 37/41 dir: use the_hash_algo for empty blob object IDbrian m. carlson, Apr 23, 2018
  39. 41/41 merge-one-file: compute empty blob object IDbrian m. carlson, Apr 23, 2018
  40. SZEDER GáborApr 24, 2018
  41. brian m. carlsonApr 24, 2018
  42. 35/41 cache-tree: use is_empty_tree_oidbrian m. carlson, Apr 23, 2018
  43. 33/41 builtin/reset: convert use of EMPTY_TREE_SHA1_BINbrian m. carlson, Apr 23, 2018
  44. 31/41 wt-status: convert two uses of EMPTY_TREE_SHA1_HEXbrian m. carlson, Apr 23, 2018
  45. Martin ÅgrenApr 24, 2018
  46. brian m. carlsonMay 1, 2018
  47. 32/41 builtin/receive-pack: convert one use of EMPTY_TREE_SHA1_HEXbrian m. carlson, Apr 23, 2018
  48. 29/41 sequencer: convert one use of EMPTY_TREE_SHA1_HEXbrian m. carlson, Apr 23, 2018
  49. 27/41 builtin/merge: switch tree functions to use object_idbrian m. carlson, Apr 23, 2018
  50. 23/41 upload-pack: replace use of several hard-coded constantsbrian m. carlson, Apr 23, 2018
  51. Simon RuderichApr 24, 2018
  52. 26/41 builtin/am: convert uses of EMPTY_TREE_SHA1_BIN to the_hash_algobrian m. carlson, Apr 23, 2018
  53. 13/41 fsck: convert static functions to struct object_idbrian m. carlson, Apr 23, 2018
  54. 24/41 diff: specify abbreviation size in terms of the_hash_algobrian m. carlson, Apr 23, 2018
  55. 19/41 commit: convert uses of get_sha1_hex to get_oid_hexbrian m. carlson, Apr 23, 2018
  56. 05/41 packfile: convert has_sha1_pack to object_idbrian m. carlson, Apr 23, 2018
  57. 18/41 index-pack: abstract away hash function constantbrian m. carlson, Apr 23, 2018
  58. Martin ÅgrenApr 24, 2018
  59. brian m. carlsonApr 24, 2018
  60. Martin ÅgrenApr 25, 2018
  61. Duy NguyenApr 26, 2018
  62. brian m. carlsonApr 27, 2018
  63. Duy NguyenApr 28, 2018
  64. 15/41 split-index: convert struct split_index to object_idbrian m. carlson, Apr 23, 2018
  65. 14/41 submodule-config: convert structures to object_idbrian m. carlson, Apr 23, 2018
  66. 12/41 tree-walk: convert get_tree_entry_follow_symlinks to object_idbrian m. carlson, Apr 23, 2018
  67. 11/41 tree-walk: avoid hard-coded 20 constantbrian m. carlson, Apr 23, 2018
  68. 10/41 pack-redundant: abstract away hash algorithmbrian m. carlson, Apr 23, 2018
  69. 08/41 packfile: abstract away hash constant valuesbrian m. carlson, Apr 23, 2018
  70. Duy NguyenMay 1, 2018
  71. brian m. carlsonMay 2, 2018
  72. Duy NguyenMay 2, 2018
  73. brian m. carlsonMay 2, 2018
  74. Duy NguyenApr 30, 2018
  75. brian m. carlsonApr 30, 2018
  76. Duy NguyenMay 1, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.