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

[PATCH 25/28] attr.c: outline the future plans by heavily commenting

From
Stefan Beller <sbeller@google.com>
Date
Oct 11, 2016, 00:21 UTC
Message-ID
<20161011002115.23312-26-sbeller@google.com>
In-Reply-To
<20161011002115.23312-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 attr.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index a08783a..e34b4c4 100644
--- a/attr.c
+++ b/attr.c
@@ -30,6 +30,11 @@ static const char git_attr__unknown[] = "(builtin)unknown";
 #define DEBUG_ATTR 0
 #endif
 
+/*
+ * NEEDSWORK: the global dictionary of the interned attributes
+ * must stay a singleton even after we become thread-ready.
+ * Access to these must be surrounded with mutex when it happens.
+ */
 struct git_attr {
 	struct git_attr *next;
 	unsigned h;
@@ -39,10 +44,19 @@ struct git_attr {
 	char name[FLEX_ARRAY];
 };
 static int attr_nr;
+static struct git_attr *(git_attr_hash[HASHSIZE]);
+
+/*
+ * NEEDSWORK: maybe-real, maybe-macro are not property of
+ * an attribute, as it depends on what .gitattributes are
+ * read.  Once we introduce per git_attr_check attr_stack
+ * and check_all_attr, the optimization based on them will
+ * become unnecessary and can go away.  So is this variable.
+ */
 static int cannot_trust_maybe_real;
 
+/* NEEDSWORK: This will become per git_attr_check */
 static struct git_attr_check_elem *check_all_attr;
-static struct git_attr *(git_attr_hash[HASHSIZE]);
 
 const char *git_attr_name(const struct git_attr *attr)
 {
@@ -117,6 +131,11 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
 	a->maybe_real = 0;
 	git_attr_hash[pos] = a;
 
+	/*
+	 * 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;
@@ -329,6 +348,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
  * .gitignore file and info/excludes file as a fallback.
  */
 
+/* NEEDSWORK: This will become per git_attr_check */
 static struct attr_stack {
 	struct attr_stack *prev;
 	char *origin;
@@ -393,6 +413,24 @@ static struct attr_stack *read_attr_from_array(const char **list)
 	return res;
 }
 
+/*
+ * NEEDSWORK: these two are tricky.  The callers assume there is a
+ * single, system-wide global state "where we read attributes from?"
+ * and when the state is flipped by calling git_attr_set_direction(),
+ * attr_stack is discarded so that subsequent attr_check will lazily
+ * read from the right place.  And they do not know or care who called
+ * by them uses the attribute subsystem, hence have no knowledge of
+ * existing git_attr_check instances or future ones that will be
+ * created).
+ *
+ * Probably we need a thread_local that holds these two variables,
+ * and a list of git_attr_check instances (which need to be maintained
+ * by hooking into git_attr_check_alloc(), git_attr_check_initl(), and
+ * git_attr_check_clear().  Then git_attr_set_direction() updates the
+ * fields in that thread_local for these two variables, iterate over
+ * all the active git_attr_check instances and discard the attr_stack
+ * they hold.  Yuck, but it sounds doable.
+ */
 static enum git_attr_direction direction;
 static struct index_state *use_index;
 
-- 
2.10.1.382.ga23ca1b.dirty
Previous: Stefan BellerNext: Stefan Beller
Message 25 of 44 in “Revamping the attr subsystem!”
  1. 00/28 Revamping the attr subsystem!Stefan Beller, Oct 11, 2016
  2. 01/28 commit.c: use strchrnul() to scan for one lineStefan Beller, Oct 11, 2016
  3. 02/28 attr.c: use strchrnul() to scan for one lineStefan Beller, Oct 11, 2016
  4. 03/28 attr.c: update a stale comment on "struct match_attr"Stefan Beller, Oct 11, 2016
  5. 07/28 attr.c: simplify macroexpand_one()Stefan Beller, Oct 11, 2016
  6. 05/28 attr.c: complete a sentence in a commentStefan Beller, Oct 11, 2016
  7. 04/28 attr.c: explain the lack of attr-name syntax check in parse_attr()Stefan Beller, Oct 11, 2016
  8. 06/28 attr.c: mark where #if DEBUG ends more clearlyStefan Beller, Oct 11, 2016
  9. 09/28 attr.c: plug small leak in parse_attr_line()Stefan Beller, Oct 11, 2016
  10. 08/28 attr.c: tighten constness around "git_attr" structureStefan Beller, Oct 11, 2016
  11. 10/28 attr: rename function and struct related to checking attributesStefan Beller, Oct 11, 2016
  12. 14/28 attr: retire git_check_attrs() APIStefan Beller, Oct 11, 2016
  13. 13/28 attr: convert git_check_attrs() callers to use the new APIStefan Beller, Oct 11, 2016
  14. 16/28 attr: add counted string version of git_attr()Stefan Beller, Oct 11, 2016
  15. 17/28 attr: expose validity check for attribute namesStefan Beller, Oct 11, 2016
  16. Brandon WilliamsOct 11, 2016
  17. Stefan BellerOct 11, 2016
  18. Brandon WilliamsOct 11, 2016
  19. Stefan BellerOct 11, 2016
  20. Brandon WilliamsOct 11, 2016
  21. 15/28 attr: add counted string version of git_check_attr()Stefan Beller, Oct 11, 2016
  22. 19/28 attr.c: add push_stack() helperStefan Beller, Oct 11, 2016
  23. 18/28 attr: support quoting pathname patterns in C styleStefan Beller, Oct 11, 2016
  24. 21/28 attr.c: rename a local variable checkStefan Beller, Oct 11, 2016
  25. 25/28 attr.c: outline the future plans by heavily commentingStefan Beller, Oct 11, 2016
  26. 20/28 attr.c: pass struct git_attr_check down the callchainStefan Beller, Oct 11, 2016
  27. 26/28 attr: make git_attr_counted staticStefan Beller, Oct 11, 2016
  28. Brandon WilliamsOct 11, 2016
  29. Stefan BellerOct 11, 2016
  30. 27/28 attr: make git_check_attr_counted staticStefan Beller, Oct 11, 2016
  31. 23/28 attr.c: introduce empty_attr_check_elems()Stefan Beller, Oct 11, 2016
  32. 28/28 attr: convert to new threadsafe APIStefan Beller, Oct 11, 2016
  33. Brandon WilliamsOct 11, 2016
  34. Junio C HamanoOct 11, 2016
  35. Stefan BellerOct 11, 2016
  36. Junio C HamanoOct 11, 2016
  37. Stefan BellerOct 11, 2016
  38. Junio C HamanoOct 11, 2016
  39. 24/28 attr.c: always pass check[] to collect_some_attrs()Stefan Beller, Oct 11, 2016
  40. 22/28 attr.c: correct ugly hack for git_all_attrs()Stefan Beller, Oct 11, 2016
  41. 11/28 attr: (re)introduce git_check_attr() and struct git_attr_checkStefan Beller, Oct 11, 2016
  42. Brandon WilliamsOct 11, 2016
  43. Junio C HamanoOct 11, 2016
  44. 12/28 attr: convert git_all_attrs() to use "struct git_attr_check"Stefan Beller, Oct 11, 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.