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

[PATCH v2 2/3] shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constant

From
Jeff King <peff@peff.net>
Date
Feb 15, 2023, 15:16 UTC
Message-ID
<Y+z3QgisHZS3l/c1@coredump.intra.peff.net>
In-Reply-To
<Y+z3MtgayoXsxaHA@coredump.intra.peff.net>

The ref_rev_parse_rules[] array is terminated with a NULL entry, and we count it and store the result in the local nr_rules variable. But we don't need to do so; since the array is a constant, we can compute its size directly. The original code probably didn't do that because it was written as part of for-each-ref, and saw the array only as a pointer. It was migrated in 7c2b3029df (make get_short_ref a public function, 2009-04-07) and could have been updated then, but that subtlety was not noticed.

We even have a constant that represents this value already, courtesy of 60650a48c0 (remote: make refspec follow the same disambiguation rule as local refs, 2018-08-01), though again, nobody noticed at the time that it could be used here, too.

The current count-up isn't a big deal, as we need to preprocess that array anyway. But it will become more cumbersome as we refactor the shortening code. So let's get rid of it and just use the constant everywhere.

Note that there are two things here that aren't just simple text replacements:

  1. We also use nr_rules to see if a previous call has initialized the
     static pre-processing variables. We can just use the scanf_fmts
     pointer to do the same thing, as it is non-NULL only after we've
     done that initialization.
  2. If nr_rules is zero after we've counted it up, we bail from the
     function. This code is unreachable, though, as the set of rules is
     hard-coded and non-empty. And that becomes even more apparent now
     that we are using the constant. So we can drop this conditional
     completely (and ironically, the code would have the same output if
     it _did_ trigger, as we'd simply skip the loop entirely and return
     the whole refname).
Signed-off-by: Jeff King <peff@peff.net>
---
 refs.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 94d938390d..84f344d8af 100644
--- a/refs.c
+++ b/refs.c
@@ -1315,11 +1315,10 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 {
 	int i;
 	static char **scanf_fmts;
-	static int nr_rules;
 	char *short_name;
 	struct strbuf resolved_buf = STRBUF_INIT;
 
-	if (!nr_rules) {
+	if (!scanf_fmts) {
 		/*
 		 * Pre-generate scanf formats from ref_rev_parse_rules[].
 		 * Generate a format suitable for scanf from a
@@ -1329,31 +1328,26 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 		size_t total_len = 0;
 		size_t offset = 0;
 
-		/* the rule list is NULL terminated, count them first */
-		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
+		for (i = 0; i < NUM_REV_PARSE_RULES; i++)
 			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
-			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
+			total_len += strlen(ref_rev_parse_rules[i]) - 2 + 1;
 
-		scanf_fmts = xmalloc(st_add(st_mult(sizeof(char *), nr_rules), total_len));
+		scanf_fmts = xmalloc(st_add(st_mult(sizeof(char *), NUM_REV_PARSE_RULES), total_len));
 
 		offset = 0;
-		for (i = 0; i < nr_rules; i++) {
+		for (i = 0; i < NUM_REV_PARSE_RULES; i++) {
 			assert(offset < total_len);
-			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules] + offset;
+			scanf_fmts[i] = (char *)&scanf_fmts[NUM_REV_PARSE_RULES] + offset;
 			offset += xsnprintf(scanf_fmts[i], total_len - offset,
 					    ref_rev_parse_rules[i], 2, "%s") + 1;
 		}
 	}
 
-	/* bail out if there are no rules */
-	if (!nr_rules)
-		return xstrdup(refname);
-
 	/* buffer for scanf result, at most refname must fit */
 	short_name = xstrdup(refname);
 
 	/* skip first rule, it will always match */
-	for (i = nr_rules - 1; i > 0 ; --i) {
+	for (i = NUM_REV_PARSE_RULES - 1; i > 0 ; --i) {
 		int j;
 		int rules_to_fail = i;
 		size_t short_name_len;
@@ -1368,7 +1362,7 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
 		 * must fail to resolve to a valid non-ambiguous ref
 		 */
 		if (strict)
-			rules_to_fail = nr_rules;
+			rules_to_fail = NUM_REV_PARSE_RULES;
 
 		/*
 		 * check if the short name resolves to a valid ref,
-- 
2.39.2.881.gb6410a20aa
Previous: Jeff KingNext: Jeff King
Message 31 of 46 in “bug report: symbolic-ref --short command echos the wrong text while use Chinese language”
  1. 孟子易Feb 13, 2023
  2. Jeff KingFeb 13, 2023
  3. Eric SunshineFeb 13, 2023
  4. Jeff KingFeb 14, 2023
  5. Eric SunshineFeb 14, 2023
  6. Eric SunshineFeb 14, 2023
  7. Junio C HamanoFeb 14, 2023
  8. Eric SunshineFeb 14, 2023
  9. Junio C HamanoFeb 14, 2023
  10. Eric SunshineFeb 14, 2023
  11. Jeff KingFeb 14, 2023
  12. Eric SunshineFeb 14, 2023
  13. Jeff KingFeb 14, 2023
  14. 0/3 get rid of sscanf() when shortening refsJeff King, Feb 14, 2023
  15. 1/3 shorten_unambiguous_ref(): avoid integer truncationJeff King, Feb 14, 2023
  16. 2/3 shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constantJeff King, Feb 14, 2023
  17. Junio C HamanoFeb 14, 2023
  18. Jeff KingFeb 14, 2023
  19. 3/3 shorten_unambiguous_ref(): avoid sscanf()Jeff King, Feb 14, 2023
  20. Junio C HamanoFeb 14, 2023
  21. Junio C HamanoFeb 14, 2023
  22. Jeff KingFeb 14, 2023
  23. Junio C HamanoFeb 14, 2023
  24. Jeff KingFeb 14, 2023
  25. Junio C HamanoFeb 15, 2023
  26. Jeff KingFeb 15, 2023
  27. Junio C HamanoFeb 15, 2023
  28. Eric SunshineFeb 14, 2023
  29. 0/3 get rid of sscanf() when shortening refsJeff King, Feb 15, 2023
  30. 1/3 shorten_unambiguous_ref(): avoid integer truncationJeff King, Feb 15, 2023
  31. 2/3 shorten_unambiguous_ref(): use NUM_REV_PARSE_RULES constantJeff King, Feb 15, 2023
  32. 3/3 shorten_unambiguous_ref(): avoid sscanf()Jeff King, Feb 15, 2023
  33. Torsten BögershausenFeb 16, 2023
  34. Eric SunshineFeb 16, 2023
  35. Junio C HamanoFeb 16, 2023
  36. Jeff KingFeb 16, 2023
  37. Junio C HamanoFeb 16, 2023
  38. Jeff KingFeb 16, 2023
  39. Torsten BögershausenFeb 17, 2023
  40. Junio C HamanoFeb 15, 2023
  41. Junio C HamanoFeb 14, 2023
  42. Jeff KingFeb 14, 2023
  43. Torsten BögershausenFeb 15, 2023
  44. Eric SunshineFeb 15, 2023
  45. Torsten BögershausenFeb 15, 2023
  46. Eric SunshineFeb 16, 2023

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.