On Thu, Sep 05, 2019 at 10:56:12AM -0700, Junio C Hamano wrote:
> Stephan Beyer <[email protected]> writes:
>
> > diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
> > index 7e79b555de..ef0963e2f4 100644
> > --- a/t/helper/test-read-cache.c
> > +++ b/t/helper/test-read-cache.c
> > @@ -4,7 +4,7 @@
> >
> > int cmd__read_cache(int argc, const char **argv)
> > {
> > - int i, cnt = 1, namelen;
> > + int i, cnt = 1, namelen = 0;
> > const char *name = NULL;
> >
> > if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
> namelen = strlen(name);
>
> The above is the only assignment to namelen in this function, and
> namelen is used like so:
>
> if (name) {
> ...
> pos = index_name_pos(&the_index, name, namelen);
>
> So somebody does not realize that skip_prefix() returns true only
> when it touches name. But skip_prefix() is inline and visible to
> the compiler, and it is quite clear that name is only touched when
> the function returns non-zero.
I said earlier that I wouldn't mind seeing "namelen = 0" here. But I
think there is a much more direct solution: keeping the assignment and
point of use closer together. That makes it more clear both to the
compiler and to a human when we expect the variable to be valid. In
fact, since it's only used once, we can drop the variable altogther. :)
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 7e79b555de..244977a29b 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c
@@ -4,11 +4,10 @@
int cmd__read_cache(int argc, const char **argv)
{
- int i, cnt = 1, namelen;
+ int i, cnt = 1;
const char *name = NULL;
if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
- namelen = strlen(name);
argc--;
argv++;
}
@@ -24,7 +23,7 @@ int cmd__read_cache(int argc, const char **argv)
refresh_index(&the_index, REFRESH_QUIET,
NULL, NULL, NULL);
- pos = index_name_pos(&the_index, name, namelen);
+ pos = index_name_pos(&the_index, name, strlen(name));
if (pos < 0)
die("%s not in index", name);
printf("%s is%s up to date\n", name,
