Re: [PATCH 3/8] grep: remove unused "prefix_length" member

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