David Chinner wrote:
On Wed, Dec 05, 2007 at 04:37:01PM +1100, Lachlan McIlroy wrote:
David Chinner wrote:
On Wed, Dec 05, 2007 at 01:11:36PM +1100, Lachlan McIlroy wrote:
David Chinner wrote:
The recent change to the internals of xfs_lowbit64 broke it on
ia32 - the code treats the 64bit value as an unsigned long
which is only 32 bits on ia32 and hence throws away the high 32
bits. 64 bit platforms are not affected.
Tested with a userspace implementation comparing the original
code with the new code.
Signed-off-by: Dave Chinner <dgc@xxxxxxx>
---
fs/xfs/xfs_bit.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: 2.6.x-xfs-new/fs/xfs/xfs_bit.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bit.h 2007-11-02
13:44:45.000000000 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_bit.h 2007-12-03 14:43:33.169851481 +1100
@@ -68,8 +68,8 @@ static inline int xfs_lowbit32(__uint32_
/* Get low bit set out of 64-bit argument, -1 if none set */
static inline int xfs_lowbit64(__uint64_t v)
{
- unsigned long t = v;
- return (v) ? find_first_bit(&t, 64) : -1;
+ unsigned long long t = v;
Why create a local copy? Why not just pass v into find_first_bit()?
Because I thought that taking the address of a function parameter
was a big no-no because the result is undefined (i.e. platform and
compiler dependent)?
Really? Didn't occur to me but then I can't say for sure I've ever
used the address of an argument. If we need a local copy then why
not use __unit64_t instead of unsigned long long?
Whatever. They are both the same.
xfs/xfs_types.h <global> 41 typedef unsigned long long int __uint64_t;
Figured they would be. Using the same type probably would have avoided
the bug in the first place. Otherwise the change is good.
|