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

[PATCH 1/6] ls-tree: don't use "show_tree_data" for "fast" callbacks

From
Teng Long <dyroneteng@gmail.com>
Date
Jan 12, 2023, 09:11 UTC
Message-ID
<20230112091135.20050-2-tenglong.tl@alibaba-inc.com>
In-Reply-To
<20230112091135.20050-1-tenglong.tl@alibaba-inc.com>
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

As noted in [1] the code that made it in as part of 9c4d58ff2c3 (ls-tree: split up "fast path" callbacks, 2022-03-23) was a "maybe a good idea, maybe not" RFC-quality patch. I hadn't looked very carefully at the resulting patterns.

The implementation shared the "struct show_tree_data data", which was introduced in e81517155e0 (ls-tree: introduce struct "show_tree_data", 2022-03-23) both for use in 455923e0a15 (ls-tree: introduce "--format" option, 2022-03-23), and because the "fat" callback hadn't been split up as 9c4d58ff2c3 did.

Now that that's been done we can see that most of what show_tree_common() was doing could be done lazily by the callbacks themselves, who in the pre-image were often using an odd mis-match of their own arguments and those same arguments stuck into the "data" structure. Let's also have the callers initialize the "type", rather than grabbing it from the "data" structure afterwards.

1. https://lore.kernel.org/git/cover-0.7-00000000000-20220310T134811Z-avarab@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Teng Long <dyronteng@gmail.com>
---
 builtin/ls-tree.c | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index c3ea09281af..cbb6782f9a5 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -173,19 +173,11 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	return recurse;
 }
 
-static int show_tree_common(struct show_tree_data *data, int *recurse,
-			    const struct object_id *oid, struct strbuf *base,
-			    const char *pathname, unsigned mode)
+static int show_tree_common(int *recurse, struct strbuf *base,
+			    const char *pathname, enum object_type type)
 {
-	enum object_type type = object_type(mode);
 	int ret = -1;
-
 	*recurse = 0;
-	data->mode = mode;
-	data->type = type;
-	data->oid = oid;
-	data->pathname = pathname;
-	data->base = base;
 
 	if (type == OBJ_BLOB) {
 		if (ls_options & LS_TREE_ONLY)
@@ -217,15 +209,15 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-	printf("%06o %s %s\t", data.mode, type_name(data.type),
-	       find_unique_abbrev(data.oid, abbrev));
-	show_tree_common_default_long(base, pathname, data.base->len);
+	printf("%06o %s %s\t", mode, type_name(object_type(mode)),
+	       find_unique_abbrev(oid, abbrev));
+	show_tree_common_default_long(base, pathname, base->len);
 	return recurse;
 }
 
@@ -235,16 +227,16 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
 	char size_text[24];
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-	if (data.type == OBJ_BLOB) {
+	if (type == OBJ_BLOB) {
 		unsigned long size;
-		if (oid_object_info(the_repository, data.oid, &size) == OBJ_BAD)
+		if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
 			xsnprintf(size_text, sizeof(size_text), "BAD");
 		else
 			xsnprintf(size_text, sizeof(size_text),
@@ -253,9 +245,9 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 		xsnprintf(size_text, sizeof(size_text), "-");
 	}
 
-	printf("%06o %s %s %7s\t", data.mode, type_name(data.type),
-	       find_unique_abbrev(data.oid, abbrev), size_text);
-	show_tree_common_default_long(base, pathname, data.base->len);
+	printf("%06o %s %s %7s\t", mode, type_name(type),
+	       find_unique_abbrev(oid, abbrev), size_text);
+	show_tree_common_default_long(base, pathname, base->len);
 	return recurse;
 }
 
@@ -266,9 +258,9 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
 	int early;
 	int recurse;
 	const size_t baselen = base->len;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
@@ -286,9 +278,9 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base,
 {
 	int early;
 	int recurse;
-	struct show_tree_data data = { 0 };
+	enum object_type type = object_type(mode);
 
-	early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
+	early = show_tree_common(&recurse, base, pathname, type);
 	if (early >= 0)
 		return early;
 
-- 
2.39.0.98.g174eaa53973.dirty
Previous: Teng LongNext: Teng Long
Message 2 of 10 in “ls-tree.c: clean-up works”
  1. 0/6 ls-tree.c: clean-up worksTeng Long, Jan 12, 2023
  2. 1/6 ls-tree: don't use "show_tree_data" for "fast" callbacksTeng Long, Jan 12, 2023
  3. 2/6 ls-tree: use a "struct options"Teng Long, Jan 12, 2023
  4. 3/6 ls-tree: fold "show_tree_data" into "cb" structTeng Long, Jan 12, 2023
  5. 4/6 ls-tree: make "line_termination" less genericTeng Long, Jan 12, 2023
  6. 5/6 ls-tree: cleanup the redundant SPACETeng Long, Jan 12, 2023
  7. Ævar Arnfjörð BjarmasonJan 12, 2023
  8. 6/6 t3104: remove shift code in 'test_ls_tree_format'Teng Long, Jan 12, 2023
  9. Ævar Arnfjörð BjarmasonJan 12, 2023
  10. Ævar Arnfjörð BjarmasonJan 12, 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.