[RFC PATCH 1/4] ls-tree: don't use "show_tree_data" for "fast" callbacks
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 17, 2022, 13:48 UTC
- Message-ID
- <RFC-patch-1.4-2d8bcfe2cab-20221117T134528Z-avarab@gmail.com>
- In-Reply-To
- <RFC-cover-0.4-00000000000-20221117T134528Z-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/ --- 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.38.0.1473.g172bcc0511c