Re: [PATCH] Fix maybe-uninitialized warnings found by gcc 9 -flto

From: Jeff King <peff@peff.net>
Date: Sep 5, 2019, 18:03 UTC
Message-ID: <20190905180348.GC23663@sigill.intra.peff.net>
In-Reply-To: <xmqqd0ger5j7.fsf@gitster-ct.c.googlers.com>

On Thu, Sep 05, 2019 at 10:56:12AM -0700, Junio C Hamano wrote:

Show 28 quoted lines

> Stephan Beyer <s-beyer@gmx.net> 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,