Multiple & vs. && and | vs. || bugs in 2.4 and 2.5

Multiple & vs. && and | vs. || bugs in 2.4 and 2.5

[Norbert Kiesel]

Hi,
inspired by the recent bug report from Muli Ben-Yehuda (see
http://marc.theaimsgroup.com/?l=linux-kernel&m=104647477025930), I
hunted for similar bugs in 2.4 and 2.5 using a simple grep:

find ${1:-.} -name \*.c | xargs grep -En '![a-zA-Z0-9_ ]+(\|[^|]|\&[^&])|([^|]\||[^&]\&) *!'

This found typos in both 2.4.20 and 2.5.63.  Please note that is a
trimmed down list of the grep, I removed what I considered false
positives (e.g. real bit operations and matches within comments).  All
but the trident.c typo looks like a logical and not a bit operation to
me.

linux-2.4.20/drivers/net/aironet4500_core.c:2679:	if (adhoc & !max_mtu)
linux-2.4.20/drivers/sound/gus_midi.c:186:	return (qlen > 0) | !(GUS_MIDI_STATUS() & MIDI_XMIT_EMPTY);
linux-2.4.20/drivers/sound/gus_wave.c:3126:	if ((gus_mem_size > 0) & !gus_no_wave_dma)
linux-2.4.20/drivers/sound/maestro.c:3362:	if(! w & PCI_STATUS_CAP_LIST) return 0;
linux-2.4.20/drivers/sound/trident.c:3063:		if(!wcontrol & 0x8000)
linux-2.4.20/drivers/video/aty128fb.c:2534:		if (!reg & LVDS_ON) {
linux-2.4.20/drivers/video/radeonfb.c:2781:		if (!lvds_gen_cntl & LVDS_ON) {
linux-2.4.20/drivers/usb/acm.c:243:	if (!urb->status & !acm->throttle)  {
linux-2.4.20/drivers/i2c/i2c-proc.c:732:			       SENSORS_ANY_I2C_BUS) & !is_isa))
linux-2.4.20/drivers/i2c/i2c-proc.c:821:			       probe[i] == SENSORS_ANY_I2C_BUS) & !is_isa))
linux-2.4.20/drivers/i2c/i2c-proc.c:838:			       SENSORS_ANY_I2C_BUS) & !is_isa))

linux-2.5.63/drivers/media/dvb/dvb-core/dvb_demux.c:224:	if (f->doneq & !neq)
linux-2.5.63/drivers/video/console/fbcon.c:459:	if (rw & !bottom_only) {
linux-2.5.63/drivers/serial/uart00.c:238:	if (!status & (UART_MSR_DCTS_MSK | UART_MSR_DDSR_MSK | 
linux-2.5.63/drivers/pnp/pnpbios/core.c:325:	if ( !boot & pnpbios_dont_use_current_config )
linux-2.5.63/drivers/pnp/pnpbios/core.c:353:	if ( !boot & pnpbios_dont_use_current_config )
linux-2.5.63/net/sctp/tsnmap.c:249:	if (started & !ended) {
linux-2.5.63/sound/oss/gus_midi.c:182:	return (qlen > 0) | !(GUS_MIDI_STATUS() & MIDI_XMIT_EMPTY);
linux-2.5.63/mm/slab.c:1646:		if (!flags & __GFP_WAIT)

Should I create patches for these and send them to the maintainers?

so long
	Norbert

Re: Multiple & vs. && and | vs. || bugs in 2.4 and 2.5

[William Lee Irwin III]

On Sun, Mar 02, 2003 at 02:27:51AM -0800, Norbert Kiesel wrote:
> Should I create patches for these and send them to the maintainers?

Yes.


-- wli

Re: Multiple & vs. && and | vs. || bugs in 2.4 and 2.5

[Muli Ben-Yehuda]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On Sun, Mar 02, 2003 at 02:27:51AM -0800, Norbert Kiesel wrote:
> Hi,
> inspired by the recent bug report from Muli Ben-Yehuda (see
> http://marc.theaimsgroup.com/?l=linux-kernel&m=104647477025930), I
> hunted for similar bugs in 2.4 and 2.5 using a simple grep:

In the interest of giving credit where credit is due, John Levon found
the bug and sent me the patch. He did this for several other drivers
as well. 

> Should I create patches for these and send them to the maintainers?

Definitely. You can ignore the 2.4.20 trident one, I'll fix it and
push it to Marcelo. 

Thanks, 
Muli. 
-- 
Muli Ben-Yehuda
http://www.mulix.org


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Multiple & vs. && and | vs. || bugs in 2.4 and 2.5

[Benjamin Herrenschmidt]

On Sun, 2003-03-02 at 11:27, Norbert Kiesel wrote:

> linux-2.4.20/drivers/video/aty128fb.c:2534:		if (!reg & LVDS_ON) {
> linux-2.4.20/drivers/video/radeonfb.c:2781:		if (!lvds_gen_cntl & LVDS_ON) {

There two are single bit tests, they should probably be written
with an additional parenthesis:

	if (!(reg & LVDS_ON))
	if (!(lvds_gen_cntl & LVDS_ON))

Though in these specific cases, I tend to prefer writing it this way

	if ((reg & LVDS_ON) == 0)
	if ((lvds_gen_cntrl & LVDS_ON) == 0)

Since I'm the author of both of the above (though I'm not the driver
maintainer), feel free to send the typo fixes to Marcelo along with
Ani Joshi (driver maintainer)

Ben.