built-ins: trust the "prefix" from run_builtin()

Change code in "builtin/grep.c" and "builtin/ls-tree.c" to trust the "prefix" passed from "run_builtin()". The "prefix" we get from setup.c is either going to be NULL or a string of length >0, never "". So we can drop the "prefix && *prefix" checks added for "builtin/grep.c" in 0d042fecf2f (git-grep: show pathnames relative to the current directory, 2006-08-11), and for "builtin/ls-tree.c" in a69dd585fca (ls-tree: chomp leading directories when run from a subdirectory, 2005-12-23). As seen in code in revision.c that was added in cd676a51367 (diff --relative: output paths as relative to the current subdirectory, 2008-02-12) we already have existing code that does away with this assertion. This makes it easier to reason about a subsequent change to the "prefix_length" code in grep.c in a subsequent commit, and since we're going to the trouble of doing that let's leave behind an assert() to promise this to any future callers. For "builtin/grep.c" it would be painful to pass the "prefix" down the callchain of: cmd_grep -> grep_tree -> grep_submodule -> grep_cache -> grep_oid -> grep_source_name So for the code that needs it in grep_source_name() let's add a "grep_prefix" variable similar to the existing "ls_tree_prefix". While at it let's move the code in cmd_ls_tree() around so that we assign to the "ls_tree_prefix" right after declaring the variables, and stop assigning to "prefix". We only subsequently used that variable later in the function after clobbering it. Let's just use our own "grep_prefix" instead. Let's also add an assert() in git.c, so that we'll make this promise about the "prefix" to any current and future callers, as well as to any readers of the code. Code history: * The strlen() in "grep.c" hasn't been used since 493b7a08d80 (grep: accept relative paths outside current working directory, 2009-09-05). When that code was added in 0d042fecf2f (git-grep: show pathnames relative to the current directory, 2006-08-11) we used the length. But since 493b7a08d80 we haven't used it for anything except a boolean check that we could have done on the "prefix" member itself. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 16, 2022 at 01:00 UTC 9725c8dda20dc9bc02b552a2333963c8cb834d1d
6 files changed +13 -13
builtin/grep.c
+8 -5
@@ -26,6 +26,8 @@
26 #include "object-store.h"
27 #include "packfile.h"
28
29 +static const char *grep_prefix;
30 +
31 static char const * const grep_usage[] = {
32 N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
33 NULL
@@ -315,11 +317,11 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,
317 strbuf_reset(out);
318
319 if (opt->null_following_name) {
318 - if (opt->relative && opt->prefix_length) {
320 + if (opt->relative && grep_prefix) {
321 struct strbuf rel_buf = STRBUF_INIT;
322 const char *rel_name =
323 relative_path(filename + tree_name_len,
322 - opt->prefix, &rel_buf);
324 + grep_prefix, &rel_buf);
325
326 if (tree_name_len)
327 strbuf_add(out, filename, tree_name_len);
@@ -332,8 +334,8 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,
334 return;
335 }
336
335 - if (opt->relative && opt->prefix_length)
336 - quote_path(filename + tree_name_len, opt->prefix, out, 0);
337 + if (opt->relative && grep_prefix)
338 + quote_path(filename + tree_name_len, grep_prefix, out, 0);
339 else
340 quote_c_style(filename + tree_name_len, out, NULL, 0);
341
@@ -962,9 +964,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
964 PARSE_OPT_NOCOMPLETE),
965 OPT_END()
966 };
967 + grep_prefix = prefix;
968
969 git_config(grep_cmd_config, NULL);
967 - grep_init(&opt, the_repository, prefix);
970 + grep_init(&opt, the_repository);
971
972 /*
973 * If there is no -- then the paths must exist in the working
builtin/ls-tree.c
+1 -1
@@ -150,7 +150,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
150
151 git_config(git_default_config, NULL);
152 ls_tree_prefix = prefix;
153 - if (prefix && *prefix)
153 + if (prefix)
154 chomp_prefix = strlen(prefix);
155
156 argc = parse_options(argc, argv, prefix, ls_tree_options,
git.c
+1
@@ -436,6 +436,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
436 } else {
437 prefix = NULL;
438 }
439 + assert(!prefix || *prefix);
440 precompose_argv_prefix(argc, argv, NULL);
441 if (use_pager == -1 && run_setup &&
442 !(p->option & DELAY_PAGER_CONFIG))
grep.c
+1 -3
@@ -139,13 +139,11 @@ int grep_config(const char *var, const char *value, void *cb)
139 * default values from the template we read the configuration
140 * information in an earlier call to git_config(grep_config).
141 */
142 -void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)
142 +void grep_init(struct grep_opt *opt, struct repository *repo)
143 {
144 *opt = grep_defaults;
145
146 opt->repo = repo;
147 - opt->prefix = prefix;
148 - opt->prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
147 opt->pattern_tail = &opt->pattern_list;
148 opt->header_tail = &opt->header_list;
149 }
grep.h
+1 -3
@@ -134,8 +134,6 @@ struct grep_opt {
134 */
135 struct repository *repo;
136
137 - const char *prefix;
138 - int prefix_length;
137 int linenum;
138 int columnnum;
139 int invert;
@@ -182,7 +180,7 @@ struct grep_opt {
180 };
181
182 int grep_config(const char *var, const char *value, void *);
185 -void grep_init(struct grep_opt *, struct repository *repo, const char *prefix);
183 +void grep_init(struct grep_opt *, struct repository *repo);
184 void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);
185
186 void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);
revision.c
+1 -1
@@ -1838,7 +1838,7 @@ void repo_init_revisions(struct repository *r,
1838 revs->commit_format = CMIT_FMT_DEFAULT;
1839 revs->expand_tabs_in_log_default = 8;
1840
1841 - grep_init(&revs->grep_filter, revs->repo, prefix);
1841 + grep_init(&revs->grep_filter, revs->repo);
1842 revs->grep_filter.status_only = 1;
1843
1844 repo_diff_setup(revs->repo, &revs->diffopt);