Re: [PATCH] One more bugfix for xfs_lowbit64

[Andi Kleen <ak@colin2.muc.de>]

On Fri, Aug 22, 2003 at 04:09:53PM -0500, Steve Lord wrote:
> On Fri, 2003-08-22 at 15:10, Andi Kleen wrote:
> > (mea culpa). The error return was broken too, it would return -2,
> > not -1 for errors. Makes no difference in the callers, they never check
> > for -1, but is still better to conform to the spec.
> > 
> > Includes the previous fix for bits > 32.
> > 
> > -Andi
> > 
> > --- linux-2.6.0test3/fs/xfs/xfs_bit.c-o	2003-05-27 03:00:41.000000000 +0200
> > +++ linux-2.6.0test3/fs/xfs/xfs_bit.c	2003-08-22 22:08:14.000000000 +0200
> > @@ -156,12 +156,12 @@
> >  {
> >  	int n;
> >  	n = ffs((unsigned)v);
> > -	if (n == 0) {
> > +	if (n < 0) {
> >  		n = ffs(v >> 32);
> >  		if (n >= 0)
> >  			n+=32;
> >  	}
> > -	return n-1;
> > +	return (n < 0) ? n : n-1;
> >  }
> >  
> >  /*
> 
> You know, on second thoughts, are you sure about that? generic_ffs
> and man ffs seem to suggest otherwise.

The i386 ffs does not agree with the man page. Manpage says 0 for 
no bit set, but i386 returns -1:

static __inline__ int ffs(int x)
{
        int r;

        __asm__("bsfl %1,%0\n\t"
                "jnz 1f\n\t"			/* zero flag set when zero */
                "movl $-1,%0\n"			/* -1 for zero */
                "1:" : "=r" (r) : "rm" (x));
        return r+1;
}

That's probably a bug in itself, but would be a bit risky to change.

If generic_ffs disagrees with that it's very bad.  That would be 
a generic bug.

It would be probably safer to use __ffs which has better defined
semantics.

-Andi