David Chinner wrote:
> 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.
I figured an experience like this was behind it.
> 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.
I'm undecided. I agree there's something aesthetically
wrong about it. Technically I'm not so sure. Maybe
a longjmp() can lead bad things happening when you do
this, I don't know. Anyway, this is pretty picky.
The change was fine.
-Alex
|