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

[PATCH v2 22/27] attr: eliminate global check_all_attr array

From
BWBrandon Williams <bmwill@google.com>
Date
Jan 23, 2017, 20:35 UTC
Message-ID
<20170123203525.185058-23-bmwill@google.com>
In-Reply-To
<20170123203525.185058-1-bmwill@google.com>

Currently there is a reliance on 'check_all_attr' which is a global array of 'attr_check_item' items which is used to store the value of each attribute during the collection process.

This patch eliminates this global and instead creates an array per 'attr_check' instance which is then used in the attribute collection process. This brings the attribute system one step closer to being thread-safe.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 attr.c | 114 +++++++++++++++++++++++++++++++++++++++++++----------------------
 attr.h |   2 ++
 2 files changed, 78 insertions(+), 38 deletions(-)
diff --git a/attr.c b/attr.c
index d2ece4eba..6d2468790 100644
--- a/attr.c
+++ b/attr.c
@@ -34,7 +34,6 @@ struct git_attr {
 	int maybe_real;
 	char name[FLEX_ARRAY]; /* attribute name */
 };
-static int attr_nr;
 
 /*
  * NEEDSWORK: maybe-real, maybe-macro are not property of
@@ -45,9 +44,6 @@ static int attr_nr;
  */
 static int cannot_trust_maybe_real;
 
-/* NEEDSWORK: This will become per git_attr_check */
-static struct attr_check_item *check_all_attr;
-
 const char *git_attr_name(const struct git_attr *attr)
 {
 	return attr->name;
@@ -143,6 +139,52 @@ static void attr_hashmap_add(struct attr_hashmap *map,
 	hashmap_add(&map->map, e);
 }
 
+/*
+ * Reallocate and reinitialize the array of all attributes (which is used in
+ * the attribute collection process) in 'check' based on the global dictionary
+ * of attributes.
+ */
+static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check)
+{
+	int i;
+
+	hashmap_lock(map);
+
+	if (map->map.size < check->all_attrs_nr)
+		die("BUG: interned attributes shouldn't be deleted");
+
+	/*
+	 * If the number of attributes in the global dictionary has increased
+	 * (or this attr_check instance doesn't have an initialized all_attrs
+	 * field), reallocate the provided attr_check instance's all_attrs
+	 * field and fill each entry with its corresponding git_attr.
+	 */
+	if (map->map.size != check->all_attrs_nr) {
+		struct attr_hash_entry *e;
+		struct hashmap_iter iter;
+		hashmap_iter_init(&map->map, &iter);
+
+		REALLOC_ARRAY(check->all_attrs, map->map.size);
+		check->all_attrs_nr = map->map.size;
+
+		while ((e = hashmap_iter_next(&iter))) {
+			const struct git_attr *a = e->value;
+			check->all_attrs[a->attr_nr].attr = a;
+		}
+	}
+
+	hashmap_unlock(map);
+
+	/*
+	 * Re-initialize every entry in check->all_attrs.
+	 * This re-initialization can live outside of the locked region since
+	 * the attribute dictionary is no longer being accessed.
+	 */
+	for (i = 0; i < check->all_attrs_nr; i++) {
+		check->all_attrs[i].value = ATTR__UNKNOWN;
+	}
+}
+
 static int attr_name_valid(const char *name, size_t namelen)
 {
 	/*
@@ -196,16 +238,6 @@ static struct git_attr *git_attr_internal(const char *name, int namelen)
 
 		attr_hashmap_add(&g_attr_hashmap, a->name, namelen, a);
 		assert(a->attr_nr == (g_attr_hashmap.map.size - 1));
-
-		/*
-		 * NEEDSWORK: per git_attr_check check_all_attr
-		 * will be initialized a lot more lazily, not
-		 * like this, and not here.
-		 */
-		REALLOC_ARRAY(check_all_attr, ++attr_nr);
-		check_all_attr[a->attr_nr].attr = a;
-		check_all_attr[a->attr_nr].value = ATTR__UNKNOWN;
-		assert(a->attr_nr == (attr_nr - 1));
 	}
 
 	hashmap_unlock(&g_attr_hashmap);
@@ -791,16 +823,16 @@ static int path_matches(const char *pathname, int pathlen,
 			      pattern, prefix, pat->patternlen, pat->flags);
 }
 
-static int macroexpand_one(int attr_nr, int rem);
+static int macroexpand_one(struct attr_check_item *all_attrs, int nr, int rem);
 
-static int fill_one(const char *what, struct match_attr *a, int rem)
+static int fill_one(const char *what, struct attr_check_item *all_attrs,
+		    struct match_attr *a, int rem)
 {
-	struct attr_check_item *check = check_all_attr;
 	int i;
 
-	for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
+	for (i = a->num_attr - 1; rem > 0 && i >= 0; i--) {
 		struct git_attr *attr = a->state[i].attr;
-		const char **n = &(check[attr->attr_nr].value);
+		const char **n = &(all_attrs[attr->attr_nr].value);
 		const char *v = a->state[i].setto;
 
 		if (*n == ATTR__UNKNOWN) {
@@ -809,14 +841,15 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 				  attr, v);
 			*n = v;
 			rem--;
-			rem = macroexpand_one(attr->attr_nr, rem);
+			rem = macroexpand_one(all_attrs, attr->attr_nr, rem);
 		}
 	}
 	return rem;
 }
 
 static int fill(const char *path, int pathlen, int basename_offset,
-		struct attr_stack *stk, int rem)
+		struct attr_stack *stk, struct attr_check_item *all_attrs,
+		int rem)
 {
 	int i;
 	const char *base = stk->origin ? stk->origin : "";
@@ -827,18 +860,18 @@ static int fill(const char *path, int pathlen, int basename_offset,
 			continue;
 		if (path_matches(path, pathlen, basename_offset,
 				 &a->u.pat, base, stk->originlen))
-			rem = fill_one("fill", a, rem);
+			rem = fill_one("fill", all_attrs, a, rem);
 	}
 	return rem;
 }
 
-static int macroexpand_one(int nr, int rem)
+static int macroexpand_one(struct attr_check_item *all_attrs, int nr, int rem)
 {
 	struct attr_stack *stk;
 	int i;
 
-	if (check_all_attr[nr].value != ATTR__TRUE ||
-	    !check_all_attr[nr].attr->maybe_macro)
+	if (all_attrs[nr].value != ATTR__TRUE ||
+	    !all_attrs[nr].attr->maybe_macro)
 		return rem;
 
 	for (stk = attr_stack; stk; stk = stk->prev) {
@@ -847,7 +880,7 @@ static int macroexpand_one(int nr, int rem)
 			if (!ma->is_macro)
 				continue;
 			if (ma->u.attr->attr_nr == nr)
-				return fill_one("expand", ma, rem);
+				return fill_one("expand", all_attrs, ma, rem);
 		}
 	}
 
@@ -855,9 +888,9 @@ 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.
+ * Collect attributes for path into the array pointed to by check->all_attrs.
+ * If check->check_nr is non-zero, only attributes in check[] are collected.
+ * Otherwise all attributes are collected.
  */
 static void collect_some_attrs(const char *path, struct attr_check *check)
 {
@@ -880,15 +913,15 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
 	}
 
 	prepare_attr_stack(path, dirlen);
-	for (i = 0; i < attr_nr; i++)
-		check_all_attr[i].value = ATTR__UNKNOWN;
+	all_attrs_init(&g_attr_hashmap, check);
+
 	if (check->check_nr && !cannot_trust_maybe_real) {
 		rem = 0;
 		for (i = 0; i < check->check_nr; i++) {
 			const struct git_attr *a = check->check[i].attr;
 			if (!a->maybe_real) {
 				struct attr_check_item *c;
-				c = check_all_attr + a->attr_nr;
+				c = check->all_attrs + a->attr_nr;
 				c->value = ATTR__UNSET;
 				rem++;
 			}
@@ -897,9 +930,9 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
 			return;
 	}
 
-	rem = attr_nr;
+	rem = check->all_attrs_nr;
 	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
-		rem = fill(path, pathlen, basename_offset, stk, rem);
+		rem = fill(path, pathlen, basename_offset, stk, check->all_attrs, rem);
 }
 
 int git_check_attr(const char *path, struct attr_check *check)
@@ -909,7 +942,8 @@ int git_check_attr(const char *path, struct attr_check *check)
 	collect_some_attrs(path, check);
 
 	for (i = 0; i < check->check_nr; i++) {
-		const char *value = check_all_attr[check->check[i].attr->attr_nr].value;
+		size_t index = check->check[i].attr->attr_nr;
+		const char *value = check->all_attrs[index].value;
 		if (value == ATTR__UNKNOWN)
 			value = ATTR__UNSET;
 		check->check[i].value = value;
@@ -925,9 +959,9 @@ void git_all_attrs(const char *path, struct attr_check *check)
 	attr_check_reset(check);
 	collect_some_attrs(path, check);
 
-	for (i = 0; i < attr_nr; i++) {
-		const char *name = check_all_attr[i].attr->name;
-		const char *value = check_all_attr[i].value;
+	for (i = 0; i < check->all_attrs_nr; i++) {
+		const char *name = check->all_attrs[i].attr->name;
+		const char *value = check->all_attrs[i].value;
 		struct attr_check_item *item;
 		if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
 			continue;
@@ -997,6 +1031,10 @@ void attr_check_clear(struct attr_check *check)
 	check->check = NULL;
 	check->check_alloc = 0;
 	check->check_nr = 0;
+
+	free(check->all_attrs);
+	check->all_attrs = NULL;
+	check->all_attrs_nr = 0;
 }
 
 void attr_check_free(struct attr_check *check)
diff --git a/attr.h b/attr.h
index 8505bca79..44b21d82c 100644
--- a/attr.h
+++ b/attr.h
@@ -33,6 +33,8 @@ struct attr_check {
 	int check_nr;
 	int check_alloc;
 	struct attr_check_item *check;
+	int all_attrs_nr;
+	struct attr_check_item *all_attrs;
 };
 
 extern struct attr_check *attr_check_alloc(void);

-- 
2.11.0.483.g087da7b7c-goog
Previous: Brandon WilliamsNext: Junio C Hamano
Message 54 of 111 in “Revamp the attribute system; another round”
  1. 00/27 Revamp the attribute system; another roundBrandon Williams, Jan 12, 2017
  2. 20/27 attr: change validity check for attribute names to use positive logicBrandon Williams, Jan 12, 2017
  3. 19/27 attr: pass struct attr_check to collect_some_attrsBrandon Williams, Jan 12, 2017
  4. 18/27 attr: retire git_check_attrs() APIBrandon Williams, Jan 12, 2017
  5. 17/27 attr: convert git_check_attrs() callers to use the new APIBrandon Williams, Jan 12, 2017
  6. 15/27 attr: (re)introduce git_check_attr() and struct attr_checkBrandon Williams, Jan 12, 2017
  7. 16/27 attr: convert git_all_attrs() to use "struct attr_check"Brandon Williams, Jan 12, 2017
  8. 23/27 attr: remove maybe-real, maybe-macro from git_attrBrandon Williams, Jan 12, 2017
  9. 24/27 attr: tighten const correctness with git_attr and match_attrBrandon Williams, Jan 12, 2017
  10. 21/27 attr: use hashmap for attribute dictionaryBrandon Williams, Jan 12, 2017
  11. Stefan BellerJan 18, 2017
  12. Brandon WilliamsJan 18, 2017
  13. 25/27 attr: store attribute stacks in hashmapBrandon Williams, Jan 12, 2017
  14. Junio C HamanoJan 13, 2017
  15. Brandon WilliamsJan 18, 2017
  16. Brandon WilliamsJan 23, 2017
  17. Stefan BellerJan 18, 2017
  18. Brandon WilliamsJan 18, 2017
  19. Stefan BellerJan 18, 2017
  20. 22/27 attr: eliminate global check_all_attr arrayBrandon Williams, Jan 12, 2017
  21. 27/27 attr: reformat git_attr_set_direction() functionBrandon Williams, Jan 12, 2017
  22. 26/27 attr: push the bare repo check into read_attr()Brandon Williams, Jan 12, 2017
  23. 13/27 attr.c: outline the future plans by heavily commentingBrandon Williams, Jan 12, 2017
  24. 12/27 Documentation: fix a typoBrandon Williams, Jan 12, 2017
  25. 01/27 commit.c: use strchrnul() to scan for one lineBrandon Williams, Jan 12, 2017
  26. 02/27 attr.c: use strchrnul() to scan for one lineBrandon Williams, Jan 12, 2017
  27. 11/27 attr.c: add push_stack() helperBrandon Williams, Jan 12, 2017
  28. 08/27 attr.c: tighten constness around "git_attr" structureBrandon Williams, Jan 12, 2017
  29. 14/27 attr: rename function and struct related to checking attributesBrandon Williams, Jan 12, 2017
  30. 06/27 attr.c: mark where #if DEBUG ends more clearlyBrandon Williams, Jan 12, 2017
  31. 07/27 attr.c: simplify macroexpand_one()Brandon Williams, Jan 12, 2017
  32. 10/27 attr: support quoting pathname patterns in C styleBrandon Williams, Jan 12, 2017
  33. 09/27 attr.c: plug small leak in parse_attr_line()Brandon Williams, Jan 12, 2017
  34. 04/27 attr.c: explain the lack of attr-name syntax check in parse_attr()Brandon Williams, Jan 12, 2017
  35. 05/27 attr.c: complete a sentence in a commentBrandon Williams, Jan 12, 2017
  36. 03/27 attr.c: update a stale comment on "struct match_attr"Brandon Williams, Jan 12, 2017
  37. Junio C HamanoJan 15, 2017
  38. Jeff KingJan 16, 2017
  39. 00/27 Revamp the attribute system; another roundBrandon Williams, Jan 23, 2017
  40. 01/27 commit.c: use strchrnul() to scan for one lineBrandon Williams, Jan 23, 2017
  41. 02/27 attr.c: use strchrnul() to scan for one lineBrandon Williams, Jan 23, 2017
  42. 03/27 attr.c: update a stale comment on "struct match_attr"Brandon Williams, Jan 23, 2017
  43. 04/27 attr.c: explain the lack of attr-name syntax check in parse_attr()Brandon Williams, Jan 23, 2017
  44. 05/27 attr.c: complete a sentence in a commentBrandon Williams, Jan 23, 2017
  45. 07/27 attr.c: simplify macroexpand_one()Brandon Williams, Jan 23, 2017
  46. 10/27 attr: support quoting pathname patterns in C styleBrandon Williams, Jan 23, 2017
  47. 12/27 Documentation: fix a typoBrandon Williams, Jan 23, 2017
  48. 13/27 attr.c: outline the future plans by heavily commentingBrandon Williams, Jan 23, 2017
  49. 09/27 attr.c: plug small leak in parse_attr_line()Brandon Williams, Jan 23, 2017
  50. 11/27 attr.c: add push_stack() helperBrandon Williams, Jan 23, 2017
  51. 14/27 attr: rename function and struct related to checking attributesBrandon Williams, Jan 23, 2017
  52. 20/27 attr: change validity check for attribute names to use positive logicBrandon Williams, Jan 23, 2017
  53. 21/27 attr: use hashmap for attribute dictionaryBrandon Williams, Jan 23, 2017
  54. 22/27 attr: eliminate global check_all_attr arrayBrandon Williams, Jan 23, 2017
  55. Junio C HamanoJan 23, 2017
  56. 16/27 attr: convert git_all_attrs() to use "struct attr_check"Brandon Williams, Jan 23, 2017
  57. 23/27 attr: remove maybe-real, maybe-macro from git_attrBrandon Williams, Jan 23, 2017
  58. 24/27 attr: tighten const correctness with git_attr and match_attrBrandon Williams, Jan 23, 2017
  59. 17/27 attr: convert git_check_attrs() callers to use the new APIBrandon Williams, Jan 23, 2017
  60. 18/27 attr: retire git_check_attrs() APIBrandon Williams, Jan 23, 2017
  61. 15/27 attr: (re)introduce git_check_attr() and struct attr_checkBrandon Williams, Jan 23, 2017
  62. 06/27 attr.c: mark where #if DEBUG ends more clearlyBrandon Williams, Jan 23, 2017
  63. 08/27 attr.c: tighten constness around "git_attr" structureBrandon Williams, Jan 23, 2017
  64. 19/27 attr: pass struct attr_check to collect_some_attrsBrandon Williams, Jan 23, 2017
  65. 26/27 attr: push the bare repo check into read_attr()Brandon Williams, Jan 23, 2017
  66. 27/27 attr: reformat git_attr_set_direction() functionBrandon Williams, Jan 23, 2017
  67. 25/27 attr: store attribute stack in attr_check structureBrandon Williams, Jan 23, 2017
  68. Junio C HamanoJan 23, 2017
  69. Brandon WilliamsJan 23, 2017
  70. Brandon WilliamsJan 24, 2017
  71. Junio C HamanoJan 24, 2017
  72. Brandon WilliamsJan 25, 2017
  73. Stefan BellerJan 25, 2017
  74. Junio C HamanoJan 25, 2017
  75. Brandon WilliamsJan 25, 2017
  76. Brandon WilliamsJan 25, 2017
  77. 00/27 Revamp the attribute system; another roundBrandon Williams, Jan 28, 2017
  78. 01/27 commit.c: use strchrnul() to scan for one lineBrandon Williams, Jan 28, 2017
  79. 02/27 attr.c: use strchrnul() to scan for one lineBrandon Williams, Jan 28, 2017
  80. 05/27 attr.c: complete a sentence in a commentBrandon Williams, Jan 28, 2017
  81. 03/27 attr.c: update a stale comment on "struct match_attr"Brandon Williams, Jan 28, 2017
  82. 07/27 attr.c: simplify macroexpand_one()Brandon Williams, Jan 28, 2017
  83. 06/27 attr.c: mark where #if DEBUG ends more clearlyBrandon Williams, Jan 28, 2017
  84. 08/27 attr.c: tighten constness around "git_attr" structureBrandon Williams, Jan 28, 2017
  85. 04/27 attr.c: explain the lack of attr-name syntax check in parse_attr()Brandon Williams, Jan 28, 2017
  86. 09/27 attr.c: plug small leak in parse_attr_line()Brandon Williams, Jan 28, 2017
  87. 10/27 attr: support quoting pathname patterns in C styleBrandon Williams, Jan 28, 2017
  88. 11/27 attr.c: add push_stack() helperBrandon Williams, Jan 28, 2017
  89. 12/27 Documentation: fix a typoBrandon Williams, Jan 28, 2017
  90. 15/27 attr: (re)introduce git_check_attr() and struct attr_checkBrandon Williams, Jan 28, 2017
  91. 15/27 attr: (re)introduce git_check_attr() and struct attr_checkBrandon Williams, Jan 30, 2017
  92. 13/27 attr.c: outline the future plans by heavily commentingBrandon Williams, Jan 28, 2017
  93. 16/27 attr: convert git_all_attrs() to use "struct attr_check"Brandon Williams, Jan 28, 2017
  94. Stefan BellerJan 28, 2017
  95. Brandon WilliamsJan 29, 2017
  96. 16/27 attr: convert git_all_attrs() to use "struct attr_check"Brandon Williams, Jan 30, 2017
  97. 14/27 attr: rename function and struct related to checking attributesBrandon Williams, Jan 28, 2017
  98. 18/27 attr: retire git_check_attrs() APIBrandon Williams, Jan 28, 2017
  99. 19/27 attr: pass struct attr_check to collect_some_attrsBrandon Williams, Jan 28, 2017
  100. 17/27 attr: convert git_check_attrs() callers to use the new APIBrandon Williams, Jan 28, 2017
  101. 22/27 attr: eliminate global check_all_attr arrayBrandon Williams, Jan 28, 2017
  102. 21/27 attr: use hashmap for attribute dictionaryBrandon Williams, Jan 28, 2017
  103. 26/27 attr: push the bare repo check into read_attr()Brandon Williams, Jan 28, 2017
  104. 25/27 attr: store attribute stack in attr_check structureBrandon Williams, Jan 28, 2017
  105. 20/27 attr: change validity check for attribute names to use positive logicBrandon Williams, Jan 28, 2017
  106. 24/27 attr: tighten const correctness with git_attr and match_attrBrandon Williams, Jan 28, 2017
  107. 27/27 attr: reformat git_attr_set_direction() functionBrandon Williams, Jan 28, 2017
  108. 23/27 attr: remove maybe-real, maybe-macro from git_attrBrandon Williams, Jan 28, 2017
  109. Junio C HamanoFeb 2, 2017
  110. Brandon WilliamsFeb 9, 2017
  111. Junio C HamanoFeb 9, 2017

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.