Re: [Patch] xfs_lowbit64 broken on ia32

[David Chinner <dgc@sgi.com>]

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@sgi.com>
> >---
> > 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)?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group