[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6524: du now uses less than half as much memory, sometimes
From: |
Jim Meyering |
Subject: |
bug#6524: du now uses less than half as much memory, sometimes |
Date: |
Wed, 07 Jul 2010 12:03:58 +0200 |
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 <address@hidden>
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
- bug#6524: du now uses less than half as much memory, sometimes, Paul Eggert, 2010/07/02
- bug#6524: du now uses less than half as much memory, sometimes, Jim Meyering, 2010/07/04
- bug#6524: du now uses less than half as much memory, sometimes, Jim Meyering, 2010/07/04
- bug#6524: du now uses less than half as much memory, sometimes, Paul Eggert, 2010/07/06
- bug#6524: du now uses less than half as much memory, sometimes, Jim Meyering, 2010/07/06
- bug#6524: du now uses less than half as much memory, sometimes, Paul Eggert, 2010/07/06
- bug#6524: du now uses less than half as much memory, sometimes,
Jim Meyering <=
- bug#6524: du now uses less than half as much memory, sometimes, Jim Meyering, 2010/07/07
- bug#6524: du now uses less than half as much memory, sometimes, Paul Eggert, 2010/07/08
- bug#6524: du now uses less than half as much memory, sometimes, Paul Eggert, 2010/07/08
bug#6524: du now uses less than half as much memory, sometimes, Pádraig Brady, 2010/07/12