| Message ID | patch-3.8-3338cc95b81-20211106T210711Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | grep: simplify & delete code by changing obscure cfg variable behavior | expand |
On Sat, Nov 06, 2021 at 10:10:49PM +0100, Ævar Arnfjörð Bjarmason wrote: > Remove the "prefix_length" member, which we compute with a strlen() on > the "prefix" argument to grep_init(), but whose strlen() hasn't been > used since 493b7a08d80 (grep: accept relative paths outside current > working directory, 2009-09-05). OK, so now we *are* relying on the assumption that prefix is either NULL or a non-empty string. I assume that the last patch was along the lines of "let's clean up this redundant check before calling strlen()" and "prepare to not call strlen() at all and just check the string itself for NULL". To be honest, I imagine that it would have been much easier to review if these two had been squashed into one, since I was a little surprised to see the line I had just been commenting on in the previous patch removed. Perhaps I should have looked a little further in the series before commenting there, but I think it would have been even easier for reviewers to see these two patches together. > When this 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. > > Before a preceding commit we also used to guard the strlen() with > "prefix && *prefix", but as that commit notes the RHS of that && chain > was also redundant. Everything in this patch looks fine to me, assuming that prefix is indeed always NULL or non-empty (which I haven't verified myself). Thanks, Taylor
diff --git a/builtin/grep.c b/builtin/grep.c index 9e34a820ad4..bd4d2107351 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -315,7 +315,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename, strbuf_reset(out); if (opt->null_following_name) { - if (opt->relative && opt->prefix_length) { + if (opt->relative && opt->prefix) { struct strbuf rel_buf = STRBUF_INIT; const char *rel_name = relative_path(filename + tree_name_len, @@ -332,7 +332,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename, return; } - if (opt->relative && opt->prefix_length) + if (opt->relative && opt->prefix) quote_path(filename + tree_name_len, opt->prefix, out, 0); else quote_c_style(filename + tree_name_len, out, NULL, 0); diff --git a/grep.c b/grep.c index 88ebc504630..755afb5f96d 100644 --- a/grep.c +++ b/grep.c @@ -145,7 +145,6 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix opt->repo = repo; opt->prefix = prefix; - opt->prefix_length = prefix ? strlen(prefix) : 0; opt->pattern_tail = &opt->pattern_list; opt->header_tail = &opt->header_list; } diff --git a/grep.h b/grep.h index 95cccb670f9..467d775b5a9 100644 --- a/grep.h +++ b/grep.h @@ -135,7 +135,6 @@ struct grep_opt { struct repository *repo; const char *prefix; - int prefix_length; int linenum; int columnnum; int invert;
Remove the "prefix_length" member, which we compute with a strlen() on the "prefix" argument to grep_init(), but whose strlen() hasn't been used since 493b7a08d80 (grep: accept relative paths outside current working directory, 2009-09-05). When this 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. Before a preceding commit we also used to guard the strlen() with "prefix && *prefix", but as that commit notes the RHS of that && chain was also redundant. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/grep.c | 4 ++-- grep.c | 1 - grep.h | 1 - 3 files changed, 2 insertions(+), 4 deletions(-)