bit testing nervousness...

Hey...

WRT all these test_bit()/set_bit() cleanups. I am a bit nervous about
these changes that are coming in...

When I see types change from "u8" or "u32" to "long" just to make <foo>_bit() work, that really makes me think that cleanup is wrong. I haven't looked closely at the recent set_bit() cleanups yet, but I am willing to bet that at least some of them are wrongly changing the size of a variable's type.

My preference would be to _eliminate_ the set_bit call and simply open-code the bitop, i.e.

	set_bit(bitnum, &foo);
become
	foo |= (1 << bitnum);

Really, for each cleanup, you need to look hard at the change and see if <foo>_bit() is being used for atomicity reasons or simply programmer preference. (and other issues like endian issues) The latter can easily be changed to open-coding.

Disclaimer, my argument is null and void if each change has been closely studied and is really correct :) However I'm guessing we all are only glancing at the changes :)

Jeff

Em Sun, Dec 08, 2002 at 10:54:00PM -0500, Jeff Garzik escreveu:

lets try to calm you then 8)

I think this can be a good idea, but in some cases, like the set_rx_mode routines (multicast) it depends on the conversion to long, so those ones should be dealt with in a different fashion, BTW, I haven't touched those ones.

Lets see:

o drivers/atm/ambassador.c ([email protected])
o drivers/atm/horizon.c ([email protected])
o drivers/char/sx.c ([email protected])
o drivers/net/lance.c ([email protected])
o drivers/net/ni65.c ([email protected])
o drivers/net/dl2k.c ([email protected], [email protected])
o drivers/net/wan/sdla_fr.c ([email protected])
o include/linux/if_wanpipe_common.h ([email protected])

only sets/resets/tests a few bits, safe, no code depends on it changing its size from 32 to 64 bits.

Humm, I was expecting a second type of changes, but I think all are safe, even on a second glance. :-)

- Arnaldo