[PATCH] Avoid gcc compiler warning

Johannes Schindelin· Dec 8, 2014, 16:38 UTC · lore

At least on this developer's MacOSX (Snow Leopard, gcc-4.2.1), GCC prints a warning that 'hash' may be used uninitialized when compiling test-hashmap that 'hash' may be used uninitialized (but GCC 4.6.3 on this developer's Ubuntu server does not report this problem).

Since hash() is called from perf_hashmap() which accepts an unchecked integer value from the command line, the warning appears to be legitimate, even if the test-hashmap command is only called from the test suite.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 test-hashmap.c | 2 ++
 1 file changed, 2 insertions(+)

Show changes to test-hashmap.c +2 −0

diff --git a/test-hashmap.c b/test-hashmap.c
index 07aa7ec..40a126d 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -62,6 +62,8 @@ static unsigned int hash(unsigned int method, unsigned int i, const char *key)
 	case HASH_METHOD_0:
 		hash = 0;
 		break;
+	default:
+		die("Unknown method: %d", method);
 	}
 
 	if (method & HASH_METHOD_X2)

-- 
2.0.0.rc3.9669.g840d1f9

Jeff King· Dec 9, 2014, 08:47 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Avoid gcc compiler warning

On Mon, Dec 08, 2014 at 05:38:59PM +0100, Johannes Schindelin wrote:

Show 8 quoted lines

> At least on this developer's MacOSX (Snow Leopard, gcc-4.2.1), GCC prints
> a warning that 'hash' may be used uninitialized when compiling
> test-hashmap that 'hash' may be used uninitialized (but GCC 4.6.3 on this
> developer's Ubuntu server does not report this problem).
> 
> Since hash() is called from perf_hashmap() which accepts an unchecked
> integer value from the command line, the warning appears to be legitimate,
> even if the test-hashmap command is only called from the test suite.

I think the older gcc is wrong; we are switching on "method & 3", which must be in the range 0-3 (and we cover all cases).

I do not mind silencing the warning anyway, but see this discussion:

  http://thread.gmane.org/gmane.comp.version-control.git/258186

The thread seems very broken in gmane, but it continues here:

  http://article.gmane.org/gmane.comp.version-control.git/258232

  http://thread.gmane.org/gmane.comp.version-control.git/258245

  http://article.gmane.org/gmane.comp.version-control.git/258264

-Peff

Johannes Schindelin· Dec 9, 2014, 09:48 UTC · re: Jeff King · lore

Re: [PATCH] Avoid gcc compiler warning

Hi Peff,

On Tue, 9 Dec 2014, Jeff King wrote:

Show 12 quoted lines

> On Mon, Dec 08, 2014 at 05:38:59PM +0100, Johannes Schindelin wrote:
> 
> > At least on this developer's MacOSX (Snow Leopard, gcc-4.2.1), GCC prints
> > a warning that 'hash' may be used uninitialized when compiling
> > test-hashmap that 'hash' may be used uninitialized (but GCC 4.6.3 on this
> > developer's Ubuntu server does not report this problem).
> > 
> > Since hash() is called from perf_hashmap() which accepts an unchecked
> > integer value from the command line, the warning appears to be legitimate,
> > even if the test-hashmap command is only called from the test suite.
> 
> I think the older gcc is wrong;

Thanks for pointing to the older thread. It just happened to make it inconvenient to develop the rather huge fsck api topic branch I submitted yesterday because my vi kept jumping to test-hashmap.c because of that compiler warning (and as you know, not everybody has the luxury of being able to upgrade their gcc *cough* *Xcode* *cough* *Snow Leopard*).

> we are switching on "method & 3", which must be in the range 0-3 (and we
> cover all cases).

That is of course correct, but still does not address the compiler warning.

Junio, you said that you would prefer the useless initialization, so here goes:

-- snipsnap --
Subject: [PATCH] Avoid gcc compiler warning

The old compiler is wrong, of course, as the switch(method & 3) statement already handles all the possible cases, but that does not help in a scenario where it is hard or impossible to upgrade to a newer compiler (e.g. being stuck on an older MacOSX and having to rely on Xcode).

So let's just initialize the variable and be done with it, it is hardly a crucial part of the code because it is only used by the test suite and invisible to the end users.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 test-hashmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to test-hashmap.c +1 −1

diff --git a/test-hashmap.c b/test-hashmap.c
index 07aa7ec..cc2891d 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -47,7 +47,7 @@ static struct test_entry *alloc_test_entry(int hash,
char *key, int klen,
 
 static unsigned int hash(unsigned int method, unsigned int i, const char
*key)
 {
-	unsigned int hash;
+	unsigned int hash = 0;
 	switch (method & 3)
 	{
 	case HASH_METHOD_FNV:

-- 
2.0.0.rc3.9669.g840d1f9

patchAvoid gcc compiler warning