From 7677417b5762df860ca454e83dc41dd9df5a9d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 17:11:30 +0800 Subject: [PATCH 1/6] ls-tree: don't use "show_tree_data" for "fast" callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- 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 c3ea09281a..cbb6782f9a 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; From 030a3d5d9e49a392b347d3d857c60fc811f6fcb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 17:11:31 +0800 Subject: [PATCH 2/6] ls-tree: use a "struct options" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As a first step towards being able to turn this code into an API some day let's change the "static" options in builtin/ls-tree.c into a "struct ls_tree_options" that can be constructed dynamically without the help of parse_options(). Because we're now using non-static variables for this we'll need to clear_pathspec() at the end of cmd_ls_tree(), least various tests start failing under SANITIZE=leak. The memory leak was already there before, now it's just being brought to the surface. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- builtin/ls-tree.c | 203 ++++++++++++++++++++++++++-------------------- 1 file changed, 116 insertions(+), 87 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index cbb6782f9a..54f7b604cb 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -14,37 +14,11 @@ #include "parse-options.h" #include "pathspec.h" -static int line_termination = '\n'; -#define LS_RECURSIVE 1 -#define LS_TREE_ONLY (1 << 1) -#define LS_SHOW_TREES (1 << 2) -static int abbrev; -static int ls_options; -static struct pathspec pathspec; -static int chomp_prefix; -static const char *ls_tree_prefix; -static const char *format; -struct show_tree_data { - unsigned mode; - enum object_type type; - const struct object_id *oid; - const char *pathname; - struct strbuf *base; -}; - static const char * const ls_tree_usage[] = { N_("git ls-tree [] [...]"), NULL }; -static enum ls_tree_cmdmode { - MODE_DEFAULT = 0, - MODE_LONG, - MODE_NAME_ONLY, - MODE_NAME_STATUS, - MODE_OBJECT_ONLY, -} cmdmode; - static void expand_objectsize(struct strbuf *line, const struct object_id *oid, const enum object_type type, unsigned int padded) { @@ -64,10 +38,39 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, } } +struct ls_tree_options { + int line_termination; + int abbrev; + enum ls_tree_path_options { + LS_RECURSIVE = 1 << 0, + LS_TREE_ONLY = 1 << 1, + LS_SHOW_TREES = 1 << 2, + } ls_options; + struct pathspec pathspec; + int chomp_prefix; + const char *ls_tree_prefix; + const char *format; +}; + +struct show_tree_data { + unsigned mode; + enum object_type type; + const struct object_id *oid; + const char *pathname; + struct strbuf *base; +}; + +struct show_tree_data_cb { + struct ls_tree_options *options; + struct show_tree_data *data; +}; + static size_t expand_show_tree(struct strbuf *sb, const char *start, void *context) { - struct show_tree_data *data = context; + struct show_tree_data_cb *wrapper = context; + struct ls_tree_options *options = wrapper->options; + struct show_tree_data *data = wrapper->data; const char *end; const char *p; unsigned int errlen; @@ -92,10 +95,10 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start, } else if (skip_prefix(start, "(objectsize)", &p)) { expand_objectsize(sb, data->oid, data->type, 0); } else if (skip_prefix(start, "(objectname)", &p)) { - strbuf_add_unique_abbrev(sb, data->oid, abbrev); + strbuf_add_unique_abbrev(sb, data->oid, options->abbrev); } else if (skip_prefix(start, "(path)", &p)) { const char *name = data->base->buf; - const char *prefix = chomp_prefix ? ls_tree_prefix : NULL; + const char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL; struct strbuf quoted = STRBUF_INIT; struct strbuf sbuf = STRBUF_INIT; strbuf_addstr(data->base, data->pathname); @@ -111,18 +114,19 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start, return len; } -static int show_recursive(const char *base, size_t baselen, const char *pathname) +static int show_recursive(struct ls_tree_options *options, const char *base, + size_t baselen, const char *pathname) { int i; - if (ls_options & LS_RECURSIVE) + if (options->ls_options & LS_RECURSIVE) return 1; - if (!pathspec.nr) + if (!options->pathspec.nr) return 0; - for (i = 0; i < pathspec.nr; i++) { - const char *spec = pathspec.items[i].match; + for (i = 0; i < options->pathspec.nr; i++) { + const char *spec = options->pathspec.items[i].match; size_t len, speclen; if (strncmp(base, spec, baselen)) @@ -142,13 +146,13 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname } static int show_tree_fmt(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, void *context UNUSED) + const char *pathname, unsigned mode, void *context) { + struct ls_tree_options *options = context; size_t baselen; int recurse = 0; struct strbuf sb = STRBUF_INIT; enum object_type type = object_type(mode); - struct show_tree_data data = { .mode = mode, .type = type, @@ -156,81 +160,89 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base, .pathname = pathname, .base = base, }; + struct show_tree_data_cb cb_data = { + .data = &data, + .options = options, + }; - if (type == OBJ_TREE && show_recursive(base->buf, base->len, pathname)) + if (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname)) recurse = READ_TREE_RECURSIVE; - if (type == OBJ_TREE && recurse && !(ls_options & LS_SHOW_TREES)) + if (type == OBJ_TREE && recurse && !(options->ls_options & LS_SHOW_TREES)) return recurse; - if (type == OBJ_BLOB && (ls_options & LS_TREE_ONLY)) + if (type == OBJ_BLOB && (options->ls_options & LS_TREE_ONLY)) return 0; baselen = base->len; - strbuf_expand(&sb, format, expand_show_tree, &data); - strbuf_addch(&sb, line_termination); + strbuf_expand(&sb, options->format, expand_show_tree, &cb_data); + strbuf_addch(&sb, options->line_termination); fwrite(sb.buf, sb.len, 1, stdout); strbuf_release(&sb); strbuf_setlen(base, baselen); return recurse; } -static int show_tree_common(int *recurse, struct strbuf *base, - const char *pathname, enum object_type type) +static int show_tree_common(struct ls_tree_options *options, int *recurse, + struct strbuf *base, const char *pathname, + enum object_type type) { int ret = -1; *recurse = 0; if (type == OBJ_BLOB) { - if (ls_options & LS_TREE_ONLY) + if (options->ls_options & LS_TREE_ONLY) ret = 0; } else if (type == OBJ_TREE && - show_recursive(base->buf, base->len, pathname)) { + show_recursive(options, base->buf, base->len, pathname)) { *recurse = READ_TREE_RECURSIVE; - if (!(ls_options & LS_SHOW_TREES)) + if (!(options->ls_options & LS_SHOW_TREES)) ret = *recurse; } return ret; } -static void show_tree_common_default_long(struct strbuf *base, +static void show_tree_common_default_long(struct ls_tree_options *options, + struct strbuf *base, const char *pathname, const size_t baselen) { strbuf_addstr(base, pathname); write_name_quoted_relative(base->buf, - chomp_prefix ? ls_tree_prefix : NULL, stdout, - line_termination); + options->chomp_prefix ? options->ls_tree_prefix : NULL, stdout, + options->line_termination); strbuf_setlen(base, baselen); } static int show_tree_default(const struct object_id *oid, struct strbuf *base, const char *pathname, unsigned mode, - void *context UNUSED) + void *context) { + struct ls_tree_options *options = context; int early; int recurse; enum object_type type = object_type(mode); - early = show_tree_common(&recurse, base, pathname, type); + early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; 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); + find_unique_abbrev(oid, options->abbrev)); + show_tree_common_default_long(options, base, pathname, base->len); return recurse; } static int show_tree_long(const struct object_id *oid, struct strbuf *base, const char *pathname, unsigned mode, - void *context UNUSED) + void *context) { + struct ls_tree_options *options = context; int early; int recurse; char size_text[24]; enum object_type type = object_type(mode); - early = show_tree_common(&recurse, base, pathname, type); + early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; @@ -246,48 +258,58 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base, } 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); + find_unique_abbrev(oid, options->abbrev), size_text); + show_tree_common_default_long(options, base, pathname, base->len); return recurse; } static int show_tree_name_only(const struct object_id *oid, struct strbuf *base, const char *pathname, unsigned mode, - void *context UNUSED) + void *context) { + struct ls_tree_options *options = context; int early; int recurse; const size_t baselen = base->len; enum object_type type = object_type(mode); - early = show_tree_common(&recurse, base, pathname, type); + early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; strbuf_addstr(base, pathname); write_name_quoted_relative(base->buf, - chomp_prefix ? ls_tree_prefix : NULL, - stdout, line_termination); + options->chomp_prefix ? options->ls_tree_prefix : NULL, + stdout, options->line_termination); strbuf_setlen(base, baselen); return recurse; } static int show_tree_object(const struct object_id *oid, struct strbuf *base, const char *pathname, unsigned mode, - void *context UNUSED) + void *context) { + struct ls_tree_options *options = context; int early; int recurse; enum object_type type = object_type(mode); - early = show_tree_common(&recurse, base, pathname, type); + early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; - printf("%s%c", find_unique_abbrev(oid, abbrev), line_termination); + printf("%s%c", find_unique_abbrev(oid, options->abbrev), options->line_termination); return recurse; } +enum ls_tree_cmdmode { + MODE_DEFAULT = 0, + MODE_LONG, + MODE_NAME_ONLY, + MODE_NAME_STATUS, + MODE_OBJECT_ONLY, +}; + struct ls_tree_cmdmode_to_fmt { enum ls_tree_cmdmode mode; const char *const fmt; @@ -327,14 +349,18 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) struct tree *tree; int i, full_tree = 0; read_tree_fn_t fn = NULL; + enum ls_tree_cmdmode cmdmode = MODE_DEFAULT; + struct ls_tree_options options = { + .line_termination = '\n', + }; const struct option ls_tree_options[] = { - OPT_BIT('d', NULL, &ls_options, N_("only show trees"), + OPT_BIT('d', NULL, &options.ls_options, N_("only show trees"), LS_TREE_ONLY), - OPT_BIT('r', NULL, &ls_options, N_("recurse into subtrees"), + OPT_BIT('r', NULL, &options.ls_options, N_("recurse into subtrees"), LS_RECURSIVE), - OPT_BIT('t', NULL, &ls_options, N_("show trees when recursing"), + OPT_BIT('t', NULL, &options.ls_options, N_("show trees when recursing"), LS_SHOW_TREES), - OPT_SET_INT('z', NULL, &line_termination, + OPT_SET_INT('z', NULL, &options.line_termination, N_("terminate entries with NUL byte"), 0), OPT_CMDMODE('l', "long", &cmdmode, N_("include object size"), MODE_LONG), @@ -344,29 +370,30 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) MODE_NAME_STATUS), OPT_CMDMODE(0, "object-only", &cmdmode, N_("list only objects"), MODE_OBJECT_ONLY), - OPT_SET_INT(0, "full-name", &chomp_prefix, + OPT_SET_INT(0, "full-name", &options.chomp_prefix, N_("use full path names"), 0), OPT_BOOL(0, "full-tree", &full_tree, N_("list entire tree; not just current directory " "(implies --full-name)")), - OPT_STRING_F(0, "format", &format, N_("format"), + OPT_STRING_F(0, "format", &options.format, N_("format"), N_("format to use for the output"), PARSE_OPT_NONEG), - OPT__ABBREV(&abbrev), + OPT__ABBREV(&options.abbrev), OPT_END() }; struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format; + int ret; git_config(git_default_config, NULL); - ls_tree_prefix = prefix; + options.ls_tree_prefix = prefix; if (prefix) - chomp_prefix = strlen(prefix); + options.chomp_prefix = strlen(prefix); argc = parse_options(argc, argv, prefix, ls_tree_options, ls_tree_usage, 0); if (full_tree) { - ls_tree_prefix = prefix = NULL; - chomp_prefix = 0; + options.ls_tree_prefix = prefix = NULL; + options.chomp_prefix = 0; } /* * We wanted to detect conflicts between --name-only and @@ -378,10 +405,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) /* -d -r should imply -t, but -d by itself should not have to. */ if ( (LS_TREE_ONLY|LS_RECURSIVE) == - ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options)) - ls_options |= LS_SHOW_TREES; + ((LS_TREE_ONLY|LS_RECURSIVE) & options.ls_options)) + options.ls_options |= LS_SHOW_TREES; - if (format && cmdmode) + if (options.format && cmdmode) usage_msg_opt( _("--format can't be combined with other format-altering options"), ls_tree_usage, ls_tree_options); @@ -396,13 +423,13 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) * cannot be lifted until it is converted to use * match_pathspec() or tree_entry_interesting() */ - parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC & - ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL), + parse_pathspec(&options.pathspec, PATHSPEC_ALL_MAGIC & + ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL), PATHSPEC_PREFER_CWD, prefix, argv + 1); - for (i = 0; i < pathspec.nr; i++) - pathspec.items[i].nowildcard_len = pathspec.items[i].len; - pathspec.has_wildcard = 0; + for (i = 0; i < options.pathspec.nr; i++) + options.pathspec.items[i].nowildcard_len = options.pathspec.items[i].len; + options.pathspec.has_wildcard = 0; tree = parse_tree_indirect(&oid); if (!tree) die("not a tree object"); @@ -412,11 +439,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) */ while (m2f) { if (!m2f->fmt) { - fn = format ? show_tree_fmt : show_tree_default; - } else if (format && !strcmp(format, m2f->fmt)) { + fn = options.format ? show_tree_fmt : show_tree_default; + } else if (options.format && !strcmp(options.format, m2f->fmt)) { cmdmode = m2f->mode; fn = m2f->fn; - } else if (!format && cmdmode == m2f->mode) { + } else if (!options.format && cmdmode == m2f->mode) { fn = m2f->fn; } else { m2f++; @@ -425,5 +452,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) break; } - return !!read_tree(the_repository, tree, &pathspec, fn, NULL); + ret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options); + clear_pathspec(&options.pathspec); + return ret; } From 65d1f6c9fa780eab5af6883e309637bddc63d75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 17:11:32 +0800 Subject: [PATCH 3/6] ls-tree: fold "show_tree_data" into "cb" struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the the preceding two commits the only user of the "show_tree_data" struct needed it along with the "options" member, let's instead fold all of that into a "show_tree_data" struct that we'll use only for that callback. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- builtin/ls-tree.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 54f7b604cb..da664eecfb 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -53,6 +53,7 @@ struct ls_tree_options { }; struct show_tree_data { + struct ls_tree_options *options; unsigned mode; enum object_type type; const struct object_id *oid; @@ -60,17 +61,11 @@ struct show_tree_data { struct strbuf *base; }; -struct show_tree_data_cb { - struct ls_tree_options *options; - struct show_tree_data *data; -}; - static size_t expand_show_tree(struct strbuf *sb, const char *start, void *context) { - struct show_tree_data_cb *wrapper = context; - struct ls_tree_options *options = wrapper->options; - struct show_tree_data *data = wrapper->data; + struct show_tree_data *data = context; + struct ls_tree_options *options = data->options; const char *end; const char *p; unsigned int errlen; @@ -153,17 +148,14 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base, int recurse = 0; struct strbuf sb = STRBUF_INIT; enum object_type type = object_type(mode); - struct show_tree_data data = { + struct show_tree_data cb_data = { + .options = options, .mode = mode, .type = type, .oid = oid, .pathname = pathname, .base = base, }; - struct show_tree_data_cb cb_data = { - .data = &data, - .options = options, - }; if (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname)) recurse = READ_TREE_RECURSIVE; From e6c75d8dd7625e47e767b6f3c38b83827639e2f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 Jan 2023 17:11:33 +0800 Subject: [PATCH 4/6] ls-tree: make "line_termination" less generic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "ls-tree" command isn't capable of ending "lines" with anything except '\n' or '\0', and in the latter case we can avoid calling write_name_quoted_relative() entirely. Let's do that, less for optimization and more for clarity, the write_name_quoted_relative() API itself does much the same thing. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- builtin/ls-tree.c | 58 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index da664eecfb..a743959f2b 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -39,7 +39,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, } struct ls_tree_options { - int line_termination; + unsigned null_termination:1; int abbrev; enum ls_tree_path_options { LS_RECURSIVE = 1 << 0, @@ -166,7 +166,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base, baselen = base->len; strbuf_expand(&sb, options->format, expand_show_tree, &cb_data); - strbuf_addch(&sb, options->line_termination); + strbuf_addch(&sb, options->null_termination ? '\0' : '\n'); fwrite(sb.buf, sb.len, 1, stdout); strbuf_release(&sb); strbuf_setlen(base, baselen); @@ -198,10 +198,22 @@ static void show_tree_common_default_long(struct ls_tree_options *options, const char *pathname, const size_t baselen) { + const char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL; + strbuf_addstr(base, pathname); - write_name_quoted_relative(base->buf, - options->chomp_prefix ? options->ls_tree_prefix : NULL, stdout, - options->line_termination); + + if (options->null_termination) { + struct strbuf sb = STRBUF_INIT; + const char *name = relative_path(base->buf, prefix, &sb); + + fputs(name, stdout); + fputc('\0', stdout); + + strbuf_release(&sb); + } else { + write_name_quoted_relative(base->buf, prefix, stdout, '\n'); + } + strbuf_setlen(base, baselen); } @@ -264,15 +276,25 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base, int recurse; const size_t baselen = base->len; enum object_type type = object_type(mode); + const char *prefix; early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; + prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL; strbuf_addstr(base, pathname); - write_name_quoted_relative(base->buf, - options->chomp_prefix ? options->ls_tree_prefix : NULL, - stdout, options->line_termination); + if (options->null_termination) { + struct strbuf sb = STRBUF_INIT; + const char *name = relative_path(base->buf, prefix, &sb); + + fputs(name, stdout); + fputc('\0', stdout); + + strbuf_release(&sb); + } else { + write_name_quoted_relative(base->buf, prefix, stdout, '\n'); + } strbuf_setlen(base, baselen); return recurse; } @@ -285,12 +307,19 @@ static int show_tree_object(const struct object_id *oid, struct strbuf *base, int early; int recurse; enum object_type type = object_type(mode); + const char *str; early = show_tree_common(options, &recurse, base, pathname, type); if (early >= 0) return early; - printf("%s%c", find_unique_abbrev(oid, options->abbrev), options->line_termination); + str = find_unique_abbrev(oid, options->abbrev); + if (options->null_termination) { + fputs(str, stdout); + fputc('\0', stdout); + } else { + puts(str); + } return recurse; } @@ -342,9 +371,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) int i, full_tree = 0; read_tree_fn_t fn = NULL; enum ls_tree_cmdmode cmdmode = MODE_DEFAULT; - struct ls_tree_options options = { - .line_termination = '\n', - }; + int null_termination = 0; + struct ls_tree_options options = { 0 }; const struct option ls_tree_options[] = { OPT_BIT('d', NULL, &options.ls_options, N_("only show trees"), LS_TREE_ONLY), @@ -352,8 +380,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) LS_RECURSIVE), OPT_BIT('t', NULL, &options.ls_options, N_("show trees when recursing"), LS_SHOW_TREES), - OPT_SET_INT('z', NULL, &options.line_termination, - N_("terminate entries with NUL byte"), 0), + OPT_BOOL('z', NULL, &null_termination, + N_("terminate entries with NUL byte")), OPT_CMDMODE('l', "long", &cmdmode, N_("include object size"), MODE_LONG), OPT_CMDMODE(0, "name-only", &cmdmode, N_("list only filenames"), @@ -383,6 +411,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, ls_tree_options, ls_tree_usage, 0); + options.null_termination = null_termination; + if (full_tree) { options.ls_tree_prefix = prefix = NULL; options.chomp_prefix = 0; From 925a7c6b6b00154be667af7a67e886cfb8d812db Mon Sep 17 00:00:00 2001 From: Teng Long Date: Thu, 12 Jan 2023 17:11:34 +0800 Subject: [PATCH 5/6] ls-tree: cleanup the redundant SPACE An redundant space was found in ls-tree.c, which is no doubt a small change, but it might be OK to make a commit on its own. Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- builtin/ls-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index a743959f2b..72eb70823d 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -14,7 +14,7 @@ #include "parse-options.h" #include "pathspec.h" -static const char * const ls_tree_usage[] = { +static const char * const ls_tree_usage[] = { N_("git ls-tree [] [...]"), NULL }; From cf4936ed749a53fbdbc0f03eb101264ab6b55e89 Mon Sep 17 00:00:00 2001 From: Teng Long Date: Thu, 12 Jan 2023 17:11:35 +0800 Subject: [PATCH 6/6] t3104: remove shift code in 'test_ls_tree_format' In t3104-ls-tree-format.sh, There is a legacy 'shift 2' code and the relevant code block no longer depends on it anymore, so let's remove it for a small cleanup. Signed-off-by: Teng Long Signed-off-by: Junio C Hamano --- t/t3104-ls-tree-format.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/t3104-ls-tree-format.sh b/t/t3104-ls-tree-format.sh index 383896667b..74053978f4 100755 --- a/t/t3104-ls-tree-format.sh +++ b/t/t3104-ls-tree-format.sh @@ -20,7 +20,6 @@ test_ls_tree_format () { format=$1 && opts=$2 && fmtopts=$3 && - shift 2 && test_expect_success "ls-tree '--format=<$format>' is like options '$opts $fmtopts'" ' git ls-tree $opts -r HEAD >expect &&