Re: [Patch] xfs_lowbit64 broken on ia32

[David Chinner <dgc@sgi.com>]

On Wed, Dec 05, 2007 at 04:14:02AM -0800, Alex Elder wrote:
> David Chinner wrote:
> . . .
> > > >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)?
> 
> I've never heard of this, and it's incorrect--at least with respect
> to standard C.  (But that's not to say in practice some compiler
> does it wrong.)  Unless it's a real (details known) problem you
> shouldn't try to work around it.

It caused me pain about 10 years ago with gcc 2.? and m68k platform,
so I've just avoided doing it ever since.

IMO, taking the address of a function parameter is also bad coding
practice because it usually indicates a bug in the code. i.e. you
should have passed a pointer to the function or you should be using
a local variable rather than abusing the function parameter in strange
ways.

Cheers,

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