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

[PATCH 05/10] get_short_sha1: refactor init of disambiguation code

From
Jeff King <peff@peff.net>
Date
Sep 26, 2016, 12:00 UTC
Message-ID
<20160926120003.oixhtotisw5xnvh4@sigill.intra.peff.net>
In-Reply-To
<20160926115720.p2yb22lcq37gboon@sigill.intra.peff.net>

The disambiguation machinery has two callers: get_short_sha1 and for_each_abbrev. Both need to repeat much of the same setup: declaring buffers, sanity-checking lengths, preparing the prefixes, etc. Let's pull that into a single init function so we can avoid repeating ourselves.

Pulling the buffers into the "struct disambiguate_state" isn't strictly necessary, but it does make things simpler for the callers, who no longer have to worry about sizing them correctly (i.e., it's an implicit requirement that the caller provide 20- and 40-byte buffers).

And while we're touching this code, we can convert any magic-number sizes to the more modern GIT_SHA1_* constants.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1_name.c | 79 +++++++++++++++++++++++++++----------------------------------
 1 file changed, 35 insertions(+), 44 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 432a308..79eb1ee 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -13,9 +13,13 @@ static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *)
 typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);
 
 struct disambiguate_state {
+	int len; /* length of prefix in hex chars */
+	char hex_pfx[GIT_SHA1_HEXSZ];
+	unsigned char bin_pfx[GIT_SHA1_RAWSZ];
+
 	disambiguate_hint_fn fn;
 	void *cb_data;
-	unsigned char candidate[20];
+	unsigned char candidate[GIT_SHA1_RAWSZ];
 	unsigned candidate_exists:1;
 	unsigned candidate_checked:1;
 	unsigned candidate_ok:1;
@@ -72,10 +76,10 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 	/* otherwise, current can be discarded and candidate is still good */
 }
 
-static void find_short_object_filename(int len, const char *hex_pfx, struct disambiguate_state *ds)
+static void find_short_object_filename(struct disambiguate_state *ds)
 {
 	struct alternate_object_database *alt;
-	char hex[40];
+	char hex[GIT_SHA1_HEXSZ];
 	static struct alternate_object_database *fakeent;
 
 	if (!fakeent) {
@@ -95,7 +99,7 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 	}
 	fakeent->next = alt_odb_list;
 
-	xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
+	xsnprintf(hex, sizeof(hex), "%.2s", ds->hex_pfx);
 	for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
 		struct dirent *de;
 		DIR *dir;
@@ -103,7 +107,7 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 		 * every alt_odb struct has 42 extra bytes after the base
 		 * for exactly this purpose
 		 */
-		xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
+		xsnprintf(alt->name, 42, "%.2s/", ds->hex_pfx);
 		dir = opendir(alt->base);
 		if (!dir)
 			continue;
@@ -113,7 +117,7 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 
 			if (strlen(de->d_name) != 38)
 				continue;
-			if (memcmp(de->d_name, hex_pfx + 2, len - 2))
+			if (memcmp(de->d_name, ds->hex_pfx + 2, ds->len - 2))
 				continue;
 			memcpy(hex + 2, de->d_name, 38);
 			if (!get_sha1_hex(hex, sha1))
@@ -138,9 +142,7 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *
 	return 1;
 }
 
-static void unique_in_pack(int len,
-			  const unsigned char *bin_pfx,
-			   struct packed_git *p,
+static void unique_in_pack(struct packed_git *p,
 			   struct disambiguate_state *ds)
 {
 	uint32_t num, last, i, first = 0;
@@ -155,7 +157,7 @@ static void unique_in_pack(int len,
 		int cmp;
 
 		current = nth_packed_object_sha1(p, mid);
-		cmp = hashcmp(bin_pfx, current);
+		cmp = hashcmp(ds->bin_pfx, current);
 		if (!cmp) {
 			first = mid;
 			break;
@@ -174,20 +176,19 @@ static void unique_in_pack(int len,
 	 */
 	for (i = first; i < num && !ds->ambiguous; i++) {
 		current = nth_packed_object_sha1(p, i);
-		if (!match_sha(len, bin_pfx, current))
+		if (!match_sha(ds->len, ds->bin_pfx, current))
 			break;
 		update_candidates(ds, current);
 	}
 }
 
-static void find_short_packed_object(int len, const unsigned char *bin_pfx,
-				     struct disambiguate_state *ds)
+static void find_short_packed_object(struct disambiguate_state *ds)
 {
 	struct packed_git *p;
 
 	prepare_packed_git();
 	for (p = packed_git; p && !ds->ambiguous; p = p->next)
-		unique_in_pack(len, bin_pfx, p, ds);
+		unique_in_pack(p, ds);
 }
 
 #define SHORT_NAME_NOT_FOUND (-1)
@@ -281,14 +282,17 @@ static int disambiguate_blob_only(const unsigned char *sha1, void *cb_data_unuse
 	return kind == OBJ_BLOB;
 }
 
-static int prepare_prefixes(const char *name, int len,
-			    unsigned char *bin_pfx,
-			    char *hex_pfx)
+static int init_object_disambiguation(const char *name, int len,
+				      struct disambiguate_state *ds)
 {
 	int i;
 
-	hashclr(bin_pfx);
-	memset(hex_pfx, 'x', 40);
+	if (len < MINIMUM_ABBREV || len > GIT_SHA1_HEXSZ)
+		return -1;
+
+	memset(ds, 0, sizeof(*ds));
+	memset(ds->hex_pfx, 'x', GIT_SHA1_HEXSZ);
+
 	for (i = 0; i < len ;i++) {
 		unsigned char c = name[i];
 		unsigned char val;
@@ -302,11 +306,14 @@ static int prepare_prefixes(const char *name, int len,
 		}
 		else
 			return -1;
-		hex_pfx[i] = c;
+		ds->hex_pfx[i] = c;
 		if (!(i & 1))
 			val <<= 4;
-		bin_pfx[i >> 1] |= val;
+		ds->bin_pfx[i >> 1] |= val;
 	}
+
+	ds->len = len;
+	prepare_alt_odb();
 	return 0;
 }
 
@@ -319,20 +326,12 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 			  unsigned flags)
 {
 	int status;
-	char hex_pfx[40];
-	unsigned char bin_pfx[20];
 	struct disambiguate_state ds;
 	int quietly = !!(flags & GET_SHA1_QUIETLY);
 
-	if (len < MINIMUM_ABBREV || len > 40)
-		return -1;
-	if (prepare_prefixes(name, len, bin_pfx, hex_pfx) < 0)
+	if (init_object_disambiguation(name, len, &ds) < 0)
 		return -1;
 
-	prepare_alt_odb();
-
-	memset(&ds, 0, sizeof(ds));
-
 	if (multiple_bits_set(flags & GET_SHA1_DISAMBIGUATORS))
 		die("BUG: multiple get_short_sha1 disambiguator flags");
 
@@ -347,36 +346,28 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 	else if (flags & GET_SHA1_BLOB)
 		ds.fn = disambiguate_blob_only;
 
-	find_short_object_filename(len, hex_pfx, &ds);
-	find_short_packed_object(len, bin_pfx, &ds);
+	find_short_object_filename(&ds);
+	find_short_packed_object(&ds);
 	status = finish_object_disambiguation(&ds, sha1);
 
 	if (!quietly && (status == SHORT_NAME_AMBIGUOUS))
-		return error("short SHA1 %.*s is ambiguous.", len, hex_pfx);
+		return error("short SHA1 %.*s is ambiguous.", ds.len, ds.hex_pfx);
 	return status;
 }
 
 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {
-	char hex_pfx[40];
-	unsigned char bin_pfx[20];
 	struct disambiguate_state ds;
-	int len = strlen(prefix);
 
-	if (len < MINIMUM_ABBREV || len > 40)
+	if (init_object_disambiguation(prefix, strlen(prefix), &ds) < 0)
 		return -1;
-	if (prepare_prefixes(prefix, len, bin_pfx, hex_pfx) < 0)
-		return -1;
-
-	prepare_alt_odb();
 
-	memset(&ds, 0, sizeof(ds));
 	ds.always_call_fn = 1;
 	ds.cb_data = cb_data;
 	ds.fn = fn;
 
-	find_short_object_filename(len, hex_pfx, &ds);
-	find_short_packed_object(len, bin_pfx, &ds);
+	find_short_object_filename(&ds);
+	find_short_packed_object(&ds);
 	return ds.ambiguous;
 }
 
-- 
2.10.0.492.g14f803f
Previous: Jeff KingNext: Jeff King
Message 16 of 111 in “Changing the default for "core.abbrev"?”
  1. Linus TorvaldsSep 26, 2016
  2. Junio C HamanoSep 26, 2016
  3. Jeff KingSep 26, 2016
  4. Junio C HamanoSep 26, 2016
  5. 0/10 helping people resolve ambiguous sha1sJeff King, Sep 26, 2016
  6. 01/10 get_sha1: detect buggy calls with multiple disambiguatorsJeff King, Sep 26, 2016
  7. Junio C HamanoSep 26, 2016
  8. Jeff KingSep 26, 2016
  9. Junio C HamanoSep 26, 2016
  10. 02/10 get_sha1: avoid repeating ourselves via ONLY_TO_DIEJeff King, Sep 26, 2016
  11. 03/10 get_sha1: propagate flags to child functionsJeff King, Sep 26, 2016
  12. 04/10 get_short_sha1: peel tags when looking for treeishJeff King, Sep 26, 2016
  13. Jeff KingSep 26, 2016
  14. Junio C HamanoSep 26, 2016
  15. Jeff KingSep 26, 2016
  16. 05/10 get_short_sha1: refactor init of disambiguation codeJeff King, Sep 26, 2016
  17. 06/10 get_short_sha1: NUL-terminate hex prefixJeff King, Sep 26, 2016
  18. Junio C HamanoSep 26, 2016
  19. Jeff KingSep 26, 2016
  20. Junio C HamanoSep 26, 2016
  21. 07/10 get_short_sha1: mark ambiguity error for translationJeff King, Sep 26, 2016
  22. 08/10 sha1_array: let callbacks interrupt iterationJeff King, Sep 26, 2016
  23. 09/10 for_each_abbrev: drop duplicate objectsJeff King, Sep 26, 2016
  24. 10/10 get_short_sha1: list ambiguous objects on errorJeff King, Sep 26, 2016
  25. Linus TorvaldsSep 26, 2016
  26. Jacob KellerSep 27, 2016
  27. Jeff KingSep 27, 2016
  28. Kyle J. McKaySep 29, 2016
  29. Jeff KingSep 29, 2016
  30. Kyle J. McKaySep 29, 2016
  31. Jeff KingSep 29, 2016
  32. Junio C HamanoSep 26, 2016
  33. Jeff KingSep 26, 2016
  34. Junio C HamanoSep 26, 2016
  35. Kyle J. McKaySep 29, 2016
  36. Jeff KingSep 29, 2016
  37. Junio C HamanoSep 29, 2016
  38. Jacob KellerSep 30, 2016
  39. core.abbrev doc: document and test the abbreviation lengthÆvar Arnfjörð Bjarmason, Feb 4, 2019
  40. Junio C HamanoFeb 4, 2019
  41. Junio C HamanoFeb 4, 2019
  42. Ævar Arnfjörð BjarmasonFeb 4, 2019
  43. Jeff KingFeb 4, 2019
  44. Ævar Arnfjörð BjarmasonFeb 4, 2019
  45. Jeff KingFeb 6, 2019
  46. Ævar Arnfjörð BjarmasonFeb 6, 2019
  47. Matthieu MoySep 26, 2016
  48. Jeff KingSep 26, 2016
  49. Kyle J. McKaySep 29, 2016
  50. Christian CouderSep 26, 2016
  51. 0/4 raising core.abbrev default to 12 hexdigitsJunio C Hamano, Sep 28, 2016
  52. 3/4 worktree: honor configuration variablesJunio C Hamano, Sep 28, 2016
  53. 4/4 core.abbrev: raise the default abbreviation to 12 hexdigitsJunio C Hamano, Sep 28, 2016
  54. SZEDER GáborSep 29, 2016
  55. Lukas FleischerSep 29, 2016
  56. Jeff KingSep 29, 2016
  57. Jeff KingSep 29, 2016
  58. Matthieu MoySep 29, 2016
  59. SZEDER GáborSep 29, 2016
  60. Johannes SixtSep 29, 2016
  61. Junio C HamanoSep 29, 2016
  62. Linus TorvaldsSep 29, 2016
  63. Linus TorvaldsSep 29, 2016
  64. Linus TorvaldsSep 29, 2016
  65. Junio C HamanoSep 29, 2016
  66. Mike HommeySep 30, 2016
  67. Linus TorvaldsSep 30, 2016
  68. Ævar Arnfjörð BjarmasonSep 30, 2016
  69. Jeff KingSep 29, 2016
  70. Linus TorvaldsSep 29, 2016
  71. Junio C HamanoSep 29, 2016
  72. Linus TorvaldsSep 29, 2016
  73. Junio C HamanoSep 29, 2016
  74. Junio C HamanoSep 29, 2016
  75. Linus TorvaldsSep 30, 2016
  76. Linus TorvaldsSep 30, 2016
  77. Linus TorvaldsSep 30, 2016
  78. Linus TorvaldsSep 30, 2016
  79. Junio C HamanoSep 30, 2016
  80. Junio C HamanoSep 30, 2016
  81. Linus TorvaldsSep 30, 2016
  82. Linus TorvaldsSep 30, 2016
  83. Junio C HamanoSep 30, 2016
  84. Junio C HamanoSep 30, 2016
  85. Junio C HamanoSep 30, 2016
  86. Linus TorvaldsSep 30, 2016
  87. Junio C HamanoSep 30, 2016
  88. Linus TorvaldsSep 30, 2016
  89. Jeff KingSep 30, 2016
  90. Linus TorvaldsSep 30, 2016
  91. Jeff KingSep 30, 2016
  92. Linus TorvaldsSep 30, 2016
  93. Junio C HamanoSep 30, 2016
  94. Junio C HamanoSep 30, 2016
  95. Jeff KingSep 30, 2016
  96. Jeff KingSep 29, 2016
  97. 2/4 t13xx: do not assume system config is emptyJunio C Hamano, Sep 28, 2016
  98. Jeff KingSep 29, 2016
  99. Junio C HamanoSep 29, 2016
  100. Jeff KingSep 29, 2016
  101. Junio C HamanoSep 29, 2016
  102. Jeff KingSep 29, 2016
  103. Junio C HamanoSep 29, 2016
  104. Junio C HamanoSep 29, 2016
  105. Jeff KingSep 29, 2016
  106. Junio C HamanoSep 29, 2016
  107. Jeff KingSep 29, 2016
  108. 1/4 config: allow customizing /etc/gitconfig locationJunio C Hamano, Sep 28, 2016
  109. Jakub NarębskiSep 29, 2016
  110. Junio C HamanoSep 29, 2016
  111. Matthieu MoySep 29, 2016

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.