ls-tree: use a "struct options"

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> Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jan 12, 2023 at 17:11 UTC 030a3d5d9e49a392b347d3d857c60fc811f6fcb9
1 file changed +116 -87
builtin/ls-tree.c
+116 -87
@@ -14,37 +14,11 @@
14 #include "parse-options.h"
15 #include "pathspec.h"
16
17 -static int line_termination = '\n';
18 -#define LS_RECURSIVE 1
19 -#define LS_TREE_ONLY (1 << 1)
20 -#define LS_SHOW_TREES (1 << 2)
21 -static int abbrev;
22 -static int ls_options;
23 -static struct pathspec pathspec;
24 -static int chomp_prefix;
25 -static const char *ls_tree_prefix;
26 -static const char *format;
27 -struct show_tree_data {
28 - unsigned mode;
29 - enum object_type type;
30 - const struct object_id *oid;
31 - const char *pathname;
32 - struct strbuf *base;
33 -};
34 -
17 static const char * const ls_tree_usage[] = {
18 N_("git ls-tree [<options>] <tree-ish> [<path>...]"),
19 NULL
20 };
21
40 -static enum ls_tree_cmdmode {
41 - MODE_DEFAULT = 0,
42 - MODE_LONG,
43 - MODE_NAME_ONLY,
44 - MODE_NAME_STATUS,
45 - MODE_OBJECT_ONLY,
46 -} cmdmode;
47 -
22 static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
23 const enum object_type type, unsigned int padded)
24 {
@@ -64,10 +38,39 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
38 }
39 }
40
41 +struct ls_tree_options {
42 + int line_termination;
43 + int abbrev;
44 + enum ls_tree_path_options {
45 + LS_RECURSIVE = 1 << 0,
46 + LS_TREE_ONLY = 1 << 1,
47 + LS_SHOW_TREES = 1 << 2,
48 + } ls_options;
49 + struct pathspec pathspec;
50 + int chomp_prefix;
51 + const char *ls_tree_prefix;
52 + const char *format;
53 +};
54 +
55 +struct show_tree_data {
56 + unsigned mode;
57 + enum object_type type;
58 + const struct object_id *oid;
59 + const char *pathname;
60 + struct strbuf *base;
61 +};
62 +
63 +struct show_tree_data_cb {
64 + struct ls_tree_options *options;
65 + struct show_tree_data *data;
66 +};
67 +
68 static size_t expand_show_tree(struct strbuf *sb, const char *start,
69 void *context)
70 {
70 - struct show_tree_data *data = context;
71 + struct show_tree_data_cb *wrapper = context;
72 + struct ls_tree_options *options = wrapper->options;
73 + struct show_tree_data *data = wrapper->data;
74 const char *end;
75 const char *p;
76 unsigned int errlen;
@@ -92,10 +95,10 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start,
95 } else if (skip_prefix(start, "(objectsize)", &p)) {
96 expand_objectsize(sb, data->oid, data->type, 0);
97 } else if (skip_prefix(start, "(objectname)", &p)) {
95 - strbuf_add_unique_abbrev(sb, data->oid, abbrev);
98 + strbuf_add_unique_abbrev(sb, data->oid, options->abbrev);
99 } else if (skip_prefix(start, "(path)", &p)) {
100 const char *name = data->base->buf;
98 - const char *prefix = chomp_prefix ? ls_tree_prefix : NULL;
101 + const char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;
102 struct strbuf quoted = STRBUF_INIT;
103 struct strbuf sbuf = STRBUF_INIT;
104 strbuf_addstr(data->base, data->pathname);
@@ -111,18 +114,19 @@ static size_t expand_show_tree(struct strbuf *sb, const char *start,
114 return len;
115 }
116
114 -static int show_recursive(const char *base, size_t baselen, const char *pathname)
117 +static int show_recursive(struct ls_tree_options *options, const char *base,
118 + size_t baselen, const char *pathname)
119 {
120 int i;
121
118 - if (ls_options & LS_RECURSIVE)
122 + if (options->ls_options & LS_RECURSIVE)
123 return 1;
124
121 - if (!pathspec.nr)
125 + if (!options->pathspec.nr)
126 return 0;
127
124 - for (i = 0; i < pathspec.nr; i++) {
125 - const char *spec = pathspec.items[i].match;
128 + for (i = 0; i < options->pathspec.nr; i++) {
129 + const char *spec = options->pathspec.items[i].match;
130 size_t len, speclen;
131
132 if (strncmp(base, spec, baselen))
@@ -142,13 +146,13 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname
146 }
147
148 static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
145 - const char *pathname, unsigned mode, void *context UNUSED)
149 + const char *pathname, unsigned mode, void *context)
150 {
151 + struct ls_tree_options *options = context;
152 size_t baselen;
153 int recurse = 0;
154 struct strbuf sb = STRBUF_INIT;
155 enum object_type type = object_type(mode);
151 -
156 struct show_tree_data data = {
157 .mode = mode,
158 .type = type,
@@ -156,81 +160,89 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
160 .pathname = pathname,
161 .base = base,
162 };
163 + struct show_tree_data_cb cb_data = {
164 + .data = &data,
165 + .options = options,
166 + };
167
160 - if (type == OBJ_TREE && show_recursive(base->buf, base->len, pathname))
168 + if (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname))
169 recurse = READ_TREE_RECURSIVE;
162 - if (type == OBJ_TREE && recurse && !(ls_options & LS_SHOW_TREES))
170 + if (type == OBJ_TREE && recurse && !(options->ls_options & LS_SHOW_TREES))
171 return recurse;
164 - if (type == OBJ_BLOB && (ls_options & LS_TREE_ONLY))
172 + if (type == OBJ_BLOB && (options->ls_options & LS_TREE_ONLY))
173 return 0;
174
175 baselen = base->len;
168 - strbuf_expand(&sb, format, expand_show_tree, &data);
169 - strbuf_addch(&sb, line_termination);
176 + strbuf_expand(&sb, options->format, expand_show_tree, &cb_data);
177 + strbuf_addch(&sb, options->line_termination);
178 fwrite(sb.buf, sb.len, 1, stdout);
179 strbuf_release(&sb);
180 strbuf_setlen(base, baselen);
181 return recurse;
182 }
183
176 -static int show_tree_common(int *recurse, struct strbuf *base,
177 - const char *pathname, enum object_type type)
184 +static int show_tree_common(struct ls_tree_options *options, int *recurse,
185 + struct strbuf *base, const char *pathname,
186 + enum object_type type)
187 {
188 int ret = -1;
189 *recurse = 0;
190
191 if (type == OBJ_BLOB) {
183 - if (ls_options & LS_TREE_ONLY)
192 + if (options->ls_options & LS_TREE_ONLY)
193 ret = 0;
194 } else if (type == OBJ_TREE &&
186 - show_recursive(base->buf, base->len, pathname)) {
195 + show_recursive(options, base->buf, base->len, pathname)) {
196 *recurse = READ_TREE_RECURSIVE;
188 - if (!(ls_options & LS_SHOW_TREES))
197 + if (!(options->ls_options & LS_SHOW_TREES))
198 ret = *recurse;
199 }
200
201 return ret;
202 }
203
195 -static void show_tree_common_default_long(struct strbuf *base,
204 +static void show_tree_common_default_long(struct ls_tree_options *options,
205 + struct strbuf *base,
206 const char *pathname,
207 const size_t baselen)
208 {
209 strbuf_addstr(base, pathname);
210 write_name_quoted_relative(base->buf,
201 - chomp_prefix ? ls_tree_prefix : NULL, stdout,
202 - line_termination);
211 + options->chomp_prefix ? options->ls_tree_prefix : NULL, stdout,
212 + options->line_termination);
213 strbuf_setlen(base, baselen);
214 }
215
216 static int show_tree_default(const struct object_id *oid, struct strbuf *base,
217 const char *pathname, unsigned mode,
208 - void *context UNUSED)
218 + void *context)
219 {
220 + struct ls_tree_options *options = context;
221 int early;
222 int recurse;
223 enum object_type type = object_type(mode);
224
214 - early = show_tree_common(&recurse, base, pathname, type);
225 + early = show_tree_common(options, &recurse, base, pathname, type);
226 if (early >= 0)
227 return early;
228
229 printf("%06o %s %s\t", mode, type_name(object_type(mode)),
219 - find_unique_abbrev(oid, abbrev));
220 - show_tree_common_default_long(base, pathname, base->len);
230 + find_unique_abbrev(oid, options->abbrev));
231 + show_tree_common_default_long(options, base, pathname, base->len);
232 return recurse;
233 }
234
235 static int show_tree_long(const struct object_id *oid, struct strbuf *base,
236 const char *pathname, unsigned mode,
226 - void *context UNUSED)
237 + void *context)
238 {
239 + struct ls_tree_options *options = context;
240 int early;
241 int recurse;
242 char size_text[24];
243 enum object_type type = object_type(mode);
244
233 - early = show_tree_common(&recurse, base, pathname, type);
245 + early = show_tree_common(options, &recurse, base, pathname, type);
246 if (early >= 0)
247 return early;
248
@@ -246,48 +258,58 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
258 }
259
260 printf("%06o %s %s %7s\t", mode, type_name(type),
249 - find_unique_abbrev(oid, abbrev), size_text);
250 - show_tree_common_default_long(base, pathname, base->len);
261 + find_unique_abbrev(oid, options->abbrev), size_text);
262 + show_tree_common_default_long(options, base, pathname, base->len);
263 return recurse;
264 }
265
266 static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
267 const char *pathname, unsigned mode,
256 - void *context UNUSED)
268 + void *context)
269 {
270 + struct ls_tree_options *options = context;
271 int early;
272 int recurse;
273 const size_t baselen = base->len;
274 enum object_type type = object_type(mode);
275
263 - early = show_tree_common(&recurse, base, pathname, type);
276 + early = show_tree_common(options, &recurse, base, pathname, type);
277 if (early >= 0)
278 return early;
279
280 strbuf_addstr(base, pathname);
281 write_name_quoted_relative(base->buf,
269 - chomp_prefix ? ls_tree_prefix : NULL,
270 - stdout, line_termination);
282 + options->chomp_prefix ? options->ls_tree_prefix : NULL,
283 + stdout, options->line_termination);
284 strbuf_setlen(base, baselen);
285 return recurse;
286 }
287
288 static int show_tree_object(const struct object_id *oid, struct strbuf *base,
289 const char *pathname, unsigned mode,
277 - void *context UNUSED)
290 + void *context)
291 {
292 + struct ls_tree_options *options = context;
293 int early;
294 int recurse;
295 enum object_type type = object_type(mode);
296
283 - early = show_tree_common(&recurse, base, pathname, type);
297 + early = show_tree_common(options, &recurse, base, pathname, type);
298 if (early >= 0)
299 return early;
300
287 - printf("%s%c", find_unique_abbrev(oid, abbrev), line_termination);
301 + printf("%s%c", find_unique_abbrev(oid, options->abbrev), options->line_termination);
302 return recurse;
303 }
304
305 +enum ls_tree_cmdmode {
306 + MODE_DEFAULT = 0,
307 + MODE_LONG,
308 + MODE_NAME_ONLY,
309 + MODE_NAME_STATUS,
310 + MODE_OBJECT_ONLY,
311 +};
312 +
313 struct ls_tree_cmdmode_to_fmt {
314 enum ls_tree_cmdmode mode;
315 const char *const fmt;
@@ -327,14 +349,18 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
349 struct tree *tree;
350 int i, full_tree = 0;
351 read_tree_fn_t fn = NULL;
352 + enum ls_tree_cmdmode cmdmode = MODE_DEFAULT;
353 + struct ls_tree_options options = {
354 + .line_termination = '\n',
355 + };
356 const struct option ls_tree_options[] = {
331 - OPT_BIT('d', NULL, &ls_options, N_("only show trees"),
357 + OPT_BIT('d', NULL, &options.ls_options, N_("only show trees"),
358 LS_TREE_ONLY),
333 - OPT_BIT('r', NULL, &ls_options, N_("recurse into subtrees"),
359 + OPT_BIT('r', NULL, &options.ls_options, N_("recurse into subtrees"),
360 LS_RECURSIVE),
335 - OPT_BIT('t', NULL, &ls_options, N_("show trees when recursing"),
361 + OPT_BIT('t', NULL, &options.ls_options, N_("show trees when recursing"),
362 LS_SHOW_TREES),
337 - OPT_SET_INT('z', NULL, &line_termination,
363 + OPT_SET_INT('z', NULL, &options.line_termination,
364 N_("terminate entries with NUL byte"), 0),
365 OPT_CMDMODE('l', "long", &cmdmode, N_("include object size"),
366 MODE_LONG),
@@ -344,29 +370,30 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
370 MODE_NAME_STATUS),
371 OPT_CMDMODE(0, "object-only", &cmdmode, N_("list only objects"),
372 MODE_OBJECT_ONLY),
347 - OPT_SET_INT(0, "full-name", &chomp_prefix,
373 + OPT_SET_INT(0, "full-name", &options.chomp_prefix,
374 N_("use full path names"), 0),
375 OPT_BOOL(0, "full-tree", &full_tree,
376 N_("list entire tree; not just current directory "
377 "(implies --full-name)")),
352 - OPT_STRING_F(0, "format", &format, N_("format"),
378 + OPT_STRING_F(0, "format", &options.format, N_("format"),
379 N_("format to use for the output"),
380 PARSE_OPT_NONEG),
355 - OPT__ABBREV(&abbrev),
381 + OPT__ABBREV(&options.abbrev),
382 OPT_END()
383 };
384 struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;
385 + int ret;
386
387 git_config(git_default_config, NULL);
361 - ls_tree_prefix = prefix;
388 + options.ls_tree_prefix = prefix;
389 if (prefix)
363 - chomp_prefix = strlen(prefix);
390 + options.chomp_prefix = strlen(prefix);
391
392 argc = parse_options(argc, argv, prefix, ls_tree_options,
393 ls_tree_usage, 0);
394 if (full_tree) {
368 - ls_tree_prefix = prefix = NULL;
369 - chomp_prefix = 0;
395 + options.ls_tree_prefix = prefix = NULL;
396 + options.chomp_prefix = 0;
397 }
398 /*
399 * 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)
405
406 /* -d -r should imply -t, but -d by itself should not have to. */
407 if ( (LS_TREE_ONLY|LS_RECURSIVE) ==
381 - ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))
382 - ls_options |= LS_SHOW_TREES;
408 + ((LS_TREE_ONLY|LS_RECURSIVE) & options.ls_options))
409 + options.ls_options |= LS_SHOW_TREES;
410
384 - if (format && cmdmode)
411 + if (options.format && cmdmode)
412 usage_msg_opt(
413 _("--format can't be combined with other format-altering options"),
414 ls_tree_usage, ls_tree_options);
@@ -396,13 +423,13 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
423 * cannot be lifted until it is converted to use
424 * match_pathspec() or tree_entry_interesting()
425 */
399 - parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC &
400 - ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
426 + parse_pathspec(&options.pathspec, PATHSPEC_ALL_MAGIC &
427 + ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
428 PATHSPEC_PREFER_CWD,
429 prefix, argv + 1);
403 - for (i = 0; i < pathspec.nr; i++)
404 - pathspec.items[i].nowildcard_len = pathspec.items[i].len;
405 - pathspec.has_wildcard = 0;
430 + for (i = 0; i < options.pathspec.nr; i++)
431 + options.pathspec.items[i].nowildcard_len = options.pathspec.items[i].len;
432 + options.pathspec.has_wildcard = 0;
433 tree = parse_tree_indirect(&oid);
434 if (!tree)
435 die("not a tree object");
@@ -412,11 +439,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
439 */
440 while (m2f) {
441 if (!m2f->fmt) {
415 - fn = format ? show_tree_fmt : show_tree_default;
416 - } else if (format && !strcmp(format, m2f->fmt)) {
442 + fn = options.format ? show_tree_fmt : show_tree_default;
443 + } else if (options.format && !strcmp(options.format, m2f->fmt)) {
444 cmdmode = m2f->mode;
445 fn = m2f->fn;
419 - } else if (!format && cmdmode == m2f->mode) {
446 + } else if (!options.format && cmdmode == m2f->mode) {
447 fn = m2f->fn;
448 } else {
449 m2f++;
@@ -425,5 +452,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
452 break;
453 }
454
428 - return !!read_tree(the_repository, tree, &pathspec, fn, NULL);
455 + ret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options);
456 + clear_pathspec(&options.pathspec);
457 + return ret;
458 }