RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Julia Lawall
> On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> 
> > Hi Julia,
> >
> > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall <julia.lawall@xxxxxxx> wrote:
> > > OK, thanks.  I was only looking at the C code.
> > >
> > > But the C code contains a loop that is followed by:
> > >
> > >         if (!size)
> > >                 return result;
> > >         tmp = *p;
> > >
> > > found_first:
> > >         tmp |= ~0UL << size;
> > >         if (tmp == ~0UL)        /* Are any bits zero? */
> > >                 return result + size;   /* Nope. */
> > >
> > > In the first return, it would seem that result == size.  Could the second
> > > one be changed to just return size?  It should not hurt performance.
> >
> > "size" may have been changed between function entry and this line.
> > So you have to store it in a temporary.
> 
> Sorry, after reflection it seems that indeed size + result is always the
> original size, so it is actually all of the code that uses >= that is
> doing something unnecessary.  == for the failure test is fine.

There is nothing wrong with defensive coding.
The 'tmp |= ~0UL << size' ensures that the return value is 'correct'
when there are no bits set.
The function could have been defined so that this wasn't needed.

If you assume that the 'no zero bits' is unlikely, then checking the
return value from ffz() could well be slightly faster.
Not that anything is likely to notice.

	David



--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux