From: "Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
To: git@vger•kernel.org
Cc: "Taylor Blau" <me@ttaylorr•com>,
"Teng Long" <dyroneteng@gmail•com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
Subject: [RFC PATCH 2/4] ls-tree: use a "struct options"
Date: Thu, 17 Nov 2022 14:48:15 +0100 [thread overview]
Message-ID: <RFC-patch-2.4-b8ec3172557-20221117T134528Z-avarab@gmail.com> (raw)
In-Reply-To: <RFC-cover-0.4-00000000000-20221117T134528Z-avarab@gmail.com>
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 <avarab@gmail•com>
---
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 cbb6782f9a5..54f7b604cb7 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 [<options>] <tree-ish> [<path>...]"),
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;
}
--
2.38.0.1473.g172bcc0511c
next prev parent reply other threads:[~2022-11-17 13:48 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 11:30 [RFC PATCH 0/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 11:30 ` [RFC PATCH 1/6] ls-tree: cleanup the redundant SPACE Teng Long
2022-11-17 11:30 ` [RFC PATCH 2/6] t3104: remove shift code in 'test_ls_tree_format' Teng Long
2022-11-17 11:30 ` [RFC PATCH 3/6] ls-tree: optimize params of 'show_tree_common_default_long()' Teng Long
2022-11-17 11:30 ` [RFC PATCH 4/6] ls-tree: improving cohension in the print code Teng Long
2022-11-17 13:53 ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 5/6] ls-tree: introduce 'match_pattern()' function Teng Long
2022-11-17 14:02 ` Ævar Arnfjörð Bjarmason
2022-11-30 9:39 ` Ævar Arnfjörð Bjarmason
2022-11-17 11:30 ` [RFC PATCH 6/6] ls-tree: introduce '--pattern' option Teng Long
2022-11-17 14:03 ` Ævar Arnfjörð Bjarmason
2022-12-12 8:32 ` Johannes Schindelin
2022-12-12 23:57 ` Junio C Hamano
2022-12-14 5:27 ` Junio C Hamano
2022-12-14 10:03 ` Ævar Arnfjörð Bjarmason
2022-12-14 10:38 ` Junio C Hamano
2023-03-27 10:37 ` win-test: unknown terminal "xterm-256color", was " Johannes Schindelin
2023-03-27 20:42 ` Junio C Hamano
2023-03-28 18:08 ` Jeff King
2023-03-28 19:31 ` Junio C Hamano
2023-03-28 19:59 ` Jeff King
2023-03-28 20:43 ` Jeff King
2023-03-28 21:05 ` Junio C Hamano
2022-11-17 13:22 ` [RFC PATCH 0/6] " Ævar Arnfjörð Bjarmason
2022-11-17 22:02 ` Taylor Blau
2022-11-21 11:41 ` Teng Long
2022-11-21 12:12 ` Ævar Arnfjörð Bjarmason
2022-11-17 13:48 ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Ævar Arnfjörð Bjarmason
2022-11-17 13:48 ` [RFC PATCH 1/4] ls-tree: don't use "show_tree_data" for "fast" callbacks Ævar Arnfjörð Bjarmason
2022-12-21 11:47 ` Teng Long
2022-11-17 13:48 ` Ævar Arnfjörð Bjarmason [this message]
2022-11-17 13:48 ` [RFC PATCH 3/4] ls-tree: fold "show_tree_data" into "cb" struct Ævar Arnfjörð Bjarmason
2022-11-17 13:48 ` [RFC PATCH 4/4] ls-tree: make "line_termination" less generic Ævar Arnfjörð Bjarmason
2022-11-21 12:00 ` [RFC PATCH 0/4] ls-tree: pass state in struct, not globals Teng Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=RFC-patch-2.4-b8ec3172557-20221117T134528Z-avarab@gmail.com \
--to=avarab@gmail$(echo .)com \
--cc=dyroneteng@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=me@ttaylorr$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox