Paul Eggert wrote: ... >> It fails to compile on x86_64 with -Werror: ... >> di-set.c:86: error: right shift count >= width of type > > That's an incorrect warning, since the code is unreachable on > that platform and the compiler should know that it's unreachable. > This bug has been present in GCC for ages, with no signs of it > ever getting fixed. In this particular case it's fairly easy > to work around with no runtime penalty assuming reasonable > optimization (except perhaps on weird hosts where > sizeof (ino_t) > 2 * sizeof (size_t)) so I installed > this further patch: > > Subject: [PATCH] du: avoid spurious warnings with 64-bit gcc -W > > Problem reported by Jim Meyering in: > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6524#74 > * gl/lib/di-set.c (di_ent_hash): Rework so that the compiler does > not incorrectly warn about shifting by 64-bits in unreachable code. > * gl/lib/ino-map.c (ino_hash): Likewise. > --- > gl/lib/di-set.c | 2 +- > gl/lib/ino-map.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gl/lib/di-set.c b/gl/lib/di-set.c > index e0e2b24..ba44bcf 100644 > --- a/gl/lib/di-set.c > +++ b/gl/lib/di-set.c > @@ -83,7 +83,7 @@ di_ent_hash (void const *x, size_t table_size) > size_t h = dev; > int i; > for (i = 1; i < sizeof dev / sizeof h + (sizeof dev % sizeof h != 0); i++) > - h ^= dev >>= CHAR_BIT * sizeof h; > + h ^= dev >> CHAR_BIT * sizeof h * i;
Good one. Thanks for the quick fix. I prefer to use unsigned types when appropriate, and adjusted a couple of other things for readability. I'll push something like the following later today. Do you know of any system on which sizeof dev_t is larger than two times sizeof size_t? I was wondering if the added (sizeof dev % sizeof h != 0) term is actually necessary anywhere. The largest dev_t I've seen is 8 bytes wide. Just curious. I know it is required, just in case. >From 52df9e9f77b6cef49f8c42ee1d17bc0fe5448ff8 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Wed, 7 Jul 2010 11:57:26 +0200 Subject: [PATCH] di-set, ino-map: adjust a type, improve readability * gl/lib/ino-map.c (ino_hash): Declare "i" as unsigned int. Use an intermediate variable for the for-loop upper bound, so it's a little more readable. Adjust comment. * gl/lib/di-set.c (di_ent_hash): Likewise. --- gl/lib/di-set.c | 10 ++++++---- gl/lib/ino-map.c | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/gl/lib/di-set.c b/gl/lib/di-set.c index ba44bcf..231c636 100644 --- a/gl/lib/di-set.c +++ b/gl/lib/di-set.c @@ -78,11 +78,13 @@ di_ent_hash (void const *x, size_t table_size) struct di_ent const *p = x; dev_t dev = p->dev; - /* Exclusive-OR the words of DEV into H. This avoids loss of info, - without using a wider % that could be quite slow. */ + /* When DEV is wider than size_t, exclusive-OR the words of DEV into H. + This avoids loss of info, without applying % to the wider type, + which could be quite slow on some systems. */ size_t h = dev; - int i; - for (i = 1; i < sizeof dev / sizeof h + (sizeof dev % sizeof h != 0); i++) + unsigned int i; + unsigned int n_words = sizeof ino / sizeof h + (sizeof ino % sizeof h != 0); + for (i = 1; i < n_words; i++) h ^= dev >> CHAR_BIT * sizeof h * i; return h % table_size; diff --git a/gl/lib/ino-map.c b/gl/lib/ino-map.c index cc9a131..f6fdd63 100644 --- a/gl/lib/ino-map.c +++ b/gl/lib/ino-map.c @@ -56,11 +56,13 @@ ino_hash (void const *x, size_t table_size) struct ino_map_ent const *p = x; ino_t ino = p->ino; - /* Exclusive-OR the words of INO into H. This avoids loss of info, - without using a wider % that could be quite slow. */ + /* When INO is wider than size_t, exclusive-OR the words of INO into H. + This avoids loss of info, without applying % to the wider type, + which could be quite slow on some systems. */ size_t h = ino; - int i; - for (i = 1; i < sizeof ino / sizeof h + (sizeof ino % sizeof h != 0); i++) + unsigned int i; + unsigned int n_words = sizeof ino / sizeof h + (sizeof ino % sizeof h != 0); + for (i = 1; i < n_words; i++) h ^= ino >> CHAR_BIT * sizeof h * i; return h % table_size; -- 1.7.2.rc1.192.g262ff
