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

[PATCHv2 12/36] attr: convert git_all_attrs() to use "struct git_attr_check"

From
Stefan Beller <sbeller@google.com>
Date
Oct 28, 2016, 18:54 UTC
Message-ID
<20161028185502.8789-13-sbeller@google.com>
In-Reply-To
<20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>

This updates the other two ways the attribute check is done via an array of "struct git_attr_check_elem" elements. These two niches appear only in "git check-attr".

 * The caller does not know offhand what attributes it wants to ask
   about and cannot use git_attr_check_initl() to prepare the
   git_attr_check structure.
 * The caller may not know what attributes it wants to ask at all,
   and instead wants to learn everything that the given path has.

Such a caller can call git_attr_check_alloc() to allocate an empty git_attr_check, and then call git_attr_check_append() to add attribute names one by one. A new attribute can be appended until git_attr_check structure is "finalized", which happens when it is used to ask for attributes for any path by calling git_check_attr() or git_all_attrs(). A git_attr_check structure that is initialized by git_attr_check_initl() is already finalized when it is returned.

I am not at all happy with the way git_all_attrs() API turned out to be, but it is only to support one niche caller ("check-attr --all"), so I'll stop here for now.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 attr.c               | 75 ++++++++++++++++++++++++++++++++++++++--------------
 attr.h               | 16 ++++++-----
 builtin/check-attr.c | 51 ++++++++++++++++++-----------------
 3 files changed, 90 insertions(+), 52 deletions(-)
diff --git a/attr.c b/attr.c
index 20d64b34f4..7ffa19875b 100644
--- a/attr.c
+++ b/attr.c
@@ -728,6 +728,11 @@ static int macroexpand_one(int nr, int rem)
 	return rem;
 }
 
+static int attr_check_is_dynamic(const struct git_attr_check *check)
+{
+	return (void *)(check->check) != (void *)(check + 1);
+}
+
 /*
  * Collect attributes for path into the array pointed to by
  * check_all_attr. If num is non-zero, only attributes in check[] are
@@ -793,32 +798,21 @@ int git_check_attrs(const char *path, int num, struct git_attr_check_elem *check
 	return 0;
 }
 
-int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check)
+void git_all_attrs(const char *path, struct git_attr_check *check)
 {
-	int i, count, j;
+	int i;
 
+	git_attr_check_clear(check);
 	collect_some_attrs(path, 0, NULL);
 
-	/* Count the number of attributes that are set. */
-	count = 0;
-	for (i = 0; i < attr_nr; i++) {
-		const char *value = check_all_attr[i].value;
-		if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
-			++count;
-	}
-	*num = count;
-	ALLOC_ARRAY(*check, count);
-	j = 0;
 	for (i = 0; i < attr_nr; i++) {
+		const char *name = check_all_attr[i].attr->name;
 		const char *value = check_all_attr[i].value;
-		if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
-			(*check)[j].attr = check_all_attr[i].attr;
-			(*check)[j].value = value;
-			++j;
-		}
+		if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
+			continue;
+		git_attr_check_append(check, git_attr(name));
+		check->check[check->check_nr - 1].value = value;
 	}
-
-	return 0;
 }
 
 void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
@@ -836,6 +830,7 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
 
 int git_check_attr(const char *path, struct git_attr_check *check)
 {
+	check->finalized = 1;
 	return git_check_attrs(path, check->check_nr, check->check);
 }
 
@@ -853,17 +848,57 @@ struct git_attr_check *git_attr_check_initl(const char *one, ...)
 	check = xcalloc(1,
 			sizeof(*check) + cnt * sizeof(*(check->check)));
 	check->check_nr = cnt;
+	check->finalized = 1;
 	check->check = (struct git_attr_check_elem *)(check + 1);
 
 	check->check[0].attr = git_attr(one);
 	va_start(params, one);
 	for (cnt = 1; cnt < check->check_nr; cnt++) {
+		struct git_attr *attr;
 		param = va_arg(params, const char *);
 		if (!param)
 			die("BUG: counted %d != ended at %d",
 			    check->check_nr, cnt);
-		check->check[cnt].attr = git_attr(param);
+		attr = git_attr(param);
+		if (!attr)
+			die("BUG: %s: not a valid attribute name", param);
+		check->check[cnt].attr = attr;
 	}
 	va_end(params);
 	return check;
 }
+
+struct git_attr_check *git_attr_check_alloc(void)
+{
+	return xcalloc(1, sizeof(struct git_attr_check));
+}
+
+struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
+						  const struct git_attr *attr)
+{
+	struct git_attr_check_elem *elem;
+	if (check->finalized)
+		die("BUG: append after git_attr_check structure is finalized");
+	if (!attr_check_is_dynamic(check))
+		die("BUG: appending to a statically initialized git_attr_check");
+	ALLOC_GROW(check->check, check->check_nr + 1, check->check_alloc);
+	elem = &check->check[check->check_nr++];
+	elem->attr = attr;
+	return elem;
+}
+
+void git_attr_check_clear(struct git_attr_check *check)
+{
+	if (!attr_check_is_dynamic(check))
+		die("BUG: clearing a statically initialized git_attr_check");
+	free(check->check);
+	check->check_nr = 0;
+	check->check_alloc = 0;
+	check->finalized = 0;
+}
+
+void git_attr_check_free(struct git_attr_check *check)
+{
+	git_attr_check_clear(check);
+	free(check);
+}
diff --git a/attr.h b/attr.h
index 3fd8690749..0d94077d78 100644
--- a/attr.h
+++ b/attr.h
@@ -30,6 +30,7 @@ struct git_attr_check_elem {
 };
 
 struct git_attr_check {
+	int finalized;
 	int check_nr;
 	int check_alloc;
 	struct git_attr_check_elem *check;
@@ -38,6 +39,12 @@ struct git_attr_check {
 extern struct git_attr_check *git_attr_check_initl(const char *, ...);
 extern int git_check_attr(const char *path, struct git_attr_check *);
 
+extern struct git_attr_check *git_attr_check_alloc(void);
+extern struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *, const struct git_attr *);
+
+extern void git_attr_check_clear(struct git_attr_check *);
+extern void git_attr_check_free(struct git_attr_check *);
+
 /*
  * Return the name of the attribute represented by the argument.  The
  * return value is a pointer to a null-delimited string that is part
@@ -48,13 +55,10 @@ extern const char *git_attr_name(const struct git_attr *);
 int git_check_attrs(const char *path, int, struct git_attr_check_elem *);
 
 /*
- * Retrieve all attributes that apply to the specified path.  *num
- * will be set to the number of attributes on the path; **check will
- * be set to point at a newly-allocated array of git_attr_check
- * objects describing the attributes and their values.  *check must be
- * free()ed by the caller.
+ * Retrieve all attributes that apply to the specified path.
+ * check holds the attributes and their values.
  */
-int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check);
+void git_all_attrs(const char *path, struct git_attr_check *check);
 
 enum git_attr_direction {
 	GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 97e3837ead..ec61476f68 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
 	OPT_END()
 };
 
-static void output_attr(int cnt, struct git_attr_check_elem *check,
-			const char *file)
+static void output_attr(struct git_attr_check *check, const char *file)
 {
 	int j;
+	int cnt = check->check_nr;
+
 	for (j = 0; j < cnt; j++) {
-		const char *value = check[j].value;
+		const char *value = check->check[j].value;
 
 		if (ATTR_TRUE(value))
 			value = "set";
@@ -42,36 +43,37 @@ static void output_attr(int cnt, struct git_attr_check_elem *check,
 			printf("%s%c" /* path */
 			       "%s%c" /* attrname */
 			       "%s%c" /* attrvalue */,
-			       file, 0, git_attr_name(check[j].attr), 0, value, 0);
+			       file, 0,
+			       git_attr_name(check->check[j].attr), 0, value, 0);
 		} else {
 			quote_c_style(file, NULL, stdout, 0);
-			printf(": %s: %s\n", git_attr_name(check[j].attr), value);
+			printf(": %s: %s\n",
+			       git_attr_name(check->check[j].attr), value);
 		}
-
 	}
 }
 
 static void check_attr(const char *prefix,
-		       int cnt, struct git_attr_check_elem *check,
+		       struct git_attr_check *check,
 		       const char *file)
 {
 	char *full_path =
 		prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
 	if (check != NULL) {
-		if (git_check_attrs(full_path, cnt, check))
-			die("git_check_attrs died");
-		output_attr(cnt, check, file);
+		if (git_check_attr(full_path, check))
+			die("git_check_attr died");
+		output_attr(check, file);
 	} else {
-		if (git_all_attrs(full_path, &cnt, &check))
-			die("git_all_attrs died");
-		output_attr(cnt, check, file);
-		free(check);
+		check = git_attr_check_alloc();
+		git_all_attrs(full_path, check);
+		output_attr(check, file);
+		git_attr_check_free(check);
 	}
 	free(full_path);
 }
 
 static void check_attr_stdin_paths(const char *prefix,
-				   int cnt, struct git_attr_check_elem *check)
+				   struct git_attr_check *check)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf unquoted = STRBUF_INIT;
@@ -85,7 +87,7 @@ static void check_attr_stdin_paths(const char *prefix,
 				die("line is badly quoted");
 			strbuf_swap(&buf, &unquoted);
 		}
-		check_attr(prefix, cnt, check, buf.buf);
+		check_attr(prefix, check, buf.buf);
 		maybe_flush_or_die(stdout, "attribute to stdout");
 	}
 	strbuf_release(&buf);
@@ -100,7 +102,7 @@ static NORETURN void error_with_usage(const char *msg)
 
 int cmd_check_attr(int argc, const char **argv, const char *prefix)
 {
-	struct git_attr_check_elem *check;
+	struct git_attr_check *check;
 	int cnt, i, doubledash, filei;
 
 	if (!is_bare_repository())
@@ -163,24 +165,21 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
 	if (all_attrs) {
 		check = NULL;
 	} else {
-		check = xcalloc(cnt, sizeof(*check));
+		check = git_attr_check_alloc();
 		for (i = 0; i < cnt; i++) {
-			const char *name;
-			struct git_attr *a;
-			name = argv[i];
-			a = git_attr(name);
+			struct git_attr *a = git_attr(argv[i]);
 			if (!a)
 				return error("%s: not a valid attribute name",
-					name);
-			check[i].attr = a;
+					     argv[i]);
+			git_attr_check_append(check, a);
 		}
 	}
 
 	if (stdin_paths)
-		check_attr_stdin_paths(prefix, cnt, check);
+		check_attr_stdin_paths(prefix, check);
 	else {
 		for (i = filei; i < argc; i++)
-			check_attr(prefix, cnt, check, argv[i]);
+			check_attr(prefix, check, argv[i]);
 		maybe_flush_or_die(stdout, "attribute to stdout");
 	}
 	return 0;
-- 
2.10.1.714.ge3da0db
Previous: Stefan BellerNext: Stefan Beller
Message 44 of 50 in “[PATCHv2 00/36] Revamp the attr subsystem!”
  1. Stefan BellerOct 28, 2016
  2. 01/36 commit.c: use strchrnul() to scan for one lineStefan Beller, Oct 28, 2016
  3. 02/36 attr.c: use strchrnul() to scan for one lineStefan Beller, Oct 28, 2016
  4. 03/36 attr.c: update a stale comment on "struct match_attr"Stefan Beller, Oct 28, 2016
  5. 04/36 attr.c: explain the lack of attr-name syntax check in parse_attr()Stefan Beller, Oct 28, 2016
  6. 06/36 attr.c: mark where #if DEBUG ends more clearlyStefan Beller, Oct 28, 2016
  7. 07/36 attr.c: simplify macroexpand_one()Stefan Beller, Oct 28, 2016
  8. 05/36 attr.c: complete a sentence in a commentStefan Beller, Oct 28, 2016
  9. 09/36 attr.c: plug small leak in parse_attr_line()Stefan Beller, Oct 28, 2016
  10. 08/36 attr.c: tighten constness around "git_attr" structureStefan Beller, Oct 28, 2016
  11. 10/36 attr: rename function and struct related to checking attributesStefan Beller, Oct 28, 2016
  12. 14/36 attr: retire git_check_attrs() APIStefan Beller, Oct 28, 2016
  13. 19/36 attr.c: add push_stack() helperStefan Beller, Oct 28, 2016
  14. 25/36 attr.c: outline the future plans by heavily commentingStefan Beller, Oct 28, 2016
  15. 18/36 attr: support quoting pathname patterns in C styleStefan Beller, Oct 28, 2016
  16. 20/36 attr.c: pass struct git_attr_check down the callchainStefan Beller, Oct 28, 2016
  17. 21/36 attr.c: rename a local variable checkStefan Beller, Oct 28, 2016
  18. 22/36 attr.c: correct ugly hack for git_all_attrs()Stefan Beller, Oct 28, 2016
  19. 28/36 attr: keep attr stack for each checkStefan Beller, Oct 28, 2016
  20. Junio C HamanoOct 28, 2016
  21. 23/36 attr.c: introduce empty_attr_check_elems()Stefan Beller, Oct 28, 2016
  22. 26/36 attr: make git_check_attr_counted staticStefan Beller, Oct 28, 2016
  23. 27/36 attr: convert to new threadsafe APIStefan Beller, Oct 28, 2016
  24. Junio C HamanoOct 28, 2016
  25. Stefan BellerOct 28, 2016
  26. Junio C HamanoOct 28, 2016
  27. Johannes SixtOct 29, 2016
  28. 29/36 Documentation: fix a typoStefan Beller, Oct 28, 2016
  29. 34/36 submodule update: add `--init-default-path` switchStefan Beller, Oct 28, 2016
  30. SQUASH to: submodule update: add `--init-default-path` switchStefan Beller, Nov 3, 2016
  31. 32/36 pathspec: allow querying for attributesStefan Beller, Oct 28, 2016
  32. Duy NguyenNov 9, 2016
  33. Stefan BellerNov 9, 2016
  34. 36/36 completion: clone can initialize specific submodulesStefan Beller, Oct 28, 2016
  35. 31/36 pathspec: move prefix check out of the inner loopStefan Beller, Oct 28, 2016
  36. 35/36 clone: add --init-submodule=<pathspec> switchStefan Beller, Oct 28, 2016
  37. 33/36 pathspec: allow escaped query valuesStefan Beller, Oct 28, 2016
  38. 30/36 pathspec: move long magic parsing out of prefix_pathspecStefan Beller, Oct 28, 2016
  39. 24/36 attr.c: always pass check[] to collect_some_attrs()Stefan Beller, Oct 28, 2016
  40. 13/36 attr: convert git_check_attrs() callers to use the new APIStefan Beller, Oct 28, 2016
  41. 17/36 attr: expose validity check for attribute namesStefan Beller, Oct 28, 2016
  42. 16/36 attr: add counted string version of git_attr()Stefan Beller, Oct 28, 2016
  43. 15/36 attr: add counted string version of git_check_attr()Stefan Beller, Oct 28, 2016
  44. 12/36 attr: convert git_all_attrs() to use "struct git_attr_check"Stefan Beller, Oct 28, 2016
  45. 11/36 attr: (re)introduce git_check_attr() and struct git_attr_checkStefan Beller, Oct 28, 2016
  46. Junio C HamanoOct 28, 2016
  47. Stefan BellerOct 28, 2016
  48. Ramsay JonesOct 28, 2016
  49. Johannes SixtNov 3, 2016
  50. Stefan BellerNov 3, 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.