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

[PATCH 4/5] attr.c: correct ugly hack for git_all_attrs()

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 8, 2016, 22:58 UTC
Message-ID
<20160608225818.726-5-gitster@pobox.com>
In-Reply-To
<20160608225818.726-1-gitster@pobox.com>
The collect_some_attrs() function has an ugly hack since

06a604e6 (attr: avoid heavy work when we know the specified attr is not defined, 2014-12-28) added an optimization that relies on the fact that the caller knows what attributes it is interested in, so that we can leave once we know the final answer for all the attributes the caller asked.

git_all_attrs() that asks "what attributes are on this path?" however does not know what attributes it is interested in, other than the vague "we are interested in all of them", which is not a very useful thing to say. As a way to disable this optimization for this caller, the said commit added a code to skip it when the caller passes a NULL for the check structure.

However, it skipped the optimization not when check is NULL, but when the number of attributes being checked is 0, which is unnecessarily pessimistic.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 attr.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/attr.c b/attr.c
index 0e61950..cdf064e 100644
--- a/attr.c
+++ b/attr.c
@@ -743,8 +743,8 @@ static int macroexpand_one(int nr, int rem)
 
 /*
  * Collect attributes for path into the array pointed to by
- * check_all_attr. If num is non-zero, only attributes in check[] are
- * collected. Otherwise all attributes are collected.
+ * check_all_attr.  If check is not NULL, only attributes in
+ * check[] are collected. Otherwise all attributes are collected.
  */
 static void collect_some_attrs(const char *path, int pathlen,
 			       struct git_attr_check *check)
@@ -754,17 +754,6 @@ static void collect_some_attrs(const char *path, int pathlen,
 	int i, rem, dirlen;
 	const char *cp, *last_slash = NULL;
 	int basename_offset;
-	int num;
-	struct git_attr_check_elem *celem;
-
-	if (!check) {
-		/* Yuck - ugly git_all_attrs() hack! */
-		celem = NULL;
-		num = 0;
-	} else {
-		celem = check->check;
-		num = check->check_nr;
-	}
 
 	for (cp = path; cp < path + pathlen; cp++) {
 		if (*cp == '/' && cp[1])
@@ -781,9 +770,12 @@ static void collect_some_attrs(const char *path, int pathlen,
 	prepare_attr_stack(path, dirlen);
 	for (i = 0; i < attr_nr; i++)
 		check_all_attr[i].value = ATTR__UNKNOWN;
-	if (num && !cannot_trust_maybe_real) {
+
+	if (check && !cannot_trust_maybe_real) {
+		struct git_attr_check_elem *celem = check->check;
+
 		rem = 0;
-		for (i = 0; i < num; i++) {
+		for (i = 0; i < check->check_nr; i++) {
 			if (!celem[i].attr->maybe_real) {
 				struct git_attr_check_elem *c;
 				c = check_all_attr + celem[i].attr->attr_nr;
@@ -791,7 +783,7 @@ static void collect_some_attrs(const char *path, int pathlen,
 				rem++;
 			}
 		}
-		if (rem == num)
+		if (rem == check->check_nr)
 			return;
 	}
 
-- 
2.9.0-rc2-262-g9161bbf
Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 13 in “A bit more "attributes"”
  1. 0/5 A bit more "attributes"Junio C Hamano, Jun 8, 2016
  2. 1/5 attr.c: add push_stack() helperJunio C Hamano, Jun 8, 2016
  3. Stefan BellerJun 8, 2016
  4. Junio C HamanoJun 9, 2016
  5. 2/5 attr.c: pass struct git_attr_check down the callchainJunio C Hamano, Jun 8, 2016
  6. Stefan BellerJun 8, 2016
  7. 3/5 fixup! d5ad6c13Junio C Hamano, Jun 8, 2016
  8. Stefan BellerJun 9, 2016
  9. Junio C HamanoJun 9, 2016
  10. 4/5 attr.c: correct ugly hack for git_all_attrs()Junio C Hamano, Jun 8, 2016
  11. 5/5 attr.c: always pass check[] to collect_some_attrs()Junio C Hamano, Jun 8, 2016
  12. Stefan BellerJun 9, 2016
  13. Junio C HamanoJun 9, 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.