[PATCH] gx-suspmod: fix "&& 0xff" typo

[PATCH] gx-suspmod: fix "&& 0xff" typo

[Alexey Dobriyan]

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 arch/i386/kernel/cpu/cpufreq/gx-suspmod.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
+++ b/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
@@ -473,7 +473,7 @@ static int __init cpufreq_gx_init(void)
 	pci_read_config_byte(params->cs55x0, PCI_MODON, &(params->on_duration));
 	pci_read_config_byte(params->cs55x0, PCI_MODOFF, &(params->off_duration));
         pci_read_config_dword(params->cs55x0, PCI_CLASS_REVISION, &class_rev);
-	params->pci_rev = class_rev && 0xff;
+	params->pci_rev = class_rev & 0xff;
 
 	if ((ret = cpufreq_register_driver(&gx_suspmod_driver))) {
 		kfree(params);

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Randy Dunlap]

On Thu, 9 Nov 2006 01:04:35 +0300 Alexey Dobriyan wrote:

> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  arch/i386/kernel/cpu/cpufreq/gx-suspmod.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> +++ b/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
> @@ -473,7 +473,7 @@ static int __init cpufreq_gx_init(void)
>  	pci_read_config_byte(params->cs55x0, PCI_MODON, &(params->on_duration));
>  	pci_read_config_byte(params->cs55x0, PCI_MODOFF, &(params->off_duration));
>          pci_read_config_dword(params->cs55x0, PCI_CLASS_REVISION, &class_rev);
> -	params->pci_rev = class_rev && 0xff;
> +	params->pci_rev = class_rev & 0xff;

Hi,
any kind of automated detection on that one?

thanks,
---
~Randy

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Dave Jones]

On Thu, Nov 09, 2006 at 01:04:35AM +0300, Alexey Dobriyan wrote:
 > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
 > ---
 > 
 >  arch/i386/kernel/cpu/cpufreq/gx-suspmod.c |    2 +-
 >  1 file changed, 1 insertion(+), 1 deletion(-)
 > 
 > --- a/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
 > +++ b/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
 > @@ -473,7 +473,7 @@ static int __init cpufreq_gx_init(void)
 >  	pci_read_config_byte(params->cs55x0, PCI_MODON, &(params->on_duration));
 >  	pci_read_config_byte(params->cs55x0, PCI_MODOFF, &(params->off_duration));
 >          pci_read_config_dword(params->cs55x0, PCI_CLASS_REVISION, &class_rev);
 > -	params->pci_rev = class_rev && 0xff;
 > +	params->pci_rev = class_rev & 0xff;
 >  
 >  	if ((ret = cpufreq_register_driver(&gx_suspmod_driver))) {
 >  		kfree(params);

Ouch, good catch. applied.

		Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Dave Jones]

On Wed, Nov 08, 2006 at 02:10:07PM -0800, Randy Dunlap wrote:
 > On Thu, 9 Nov 2006 01:04:35 +0300 Alexey Dobriyan wrote:
 > 
 > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
 > > ---
 > > 
 > >  arch/i386/kernel/cpu/cpufreq/gx-suspmod.c |    2 +-
 > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > > 
 > > --- a/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
 > > +++ b/arch/i386/kernel/cpu/cpufreq/gx-suspmod.c
 > > @@ -473,7 +473,7 @@ static int __init cpufreq_gx_init(void)
 > >  	pci_read_config_byte(params->cs55x0, PCI_MODON, &(params->on_duration));
 > >  	pci_read_config_byte(params->cs55x0, PCI_MODOFF, &(params->off_duration));
 > >          pci_read_config_dword(params->cs55x0, PCI_CLASS_REVISION, &class_rev);
 > > -	params->pci_rev = class_rev && 0xff;
 > > +	params->pci_rev = class_rev & 0xff;
 > 
 > Hi,
 > any kind of automated detection on that one?

grep -r "&& 0x" .  seems to be pretty effective modulo
some false-positives.

		Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Alexey Dobriyan]

>  > > -	params->pci_rev = class_rev && 0xff;
>  > > +	params->pci_rev = class_rev & 0xff;
>  >
>  > Hi,
>  > any kind of automated detection on that one?
>
> grep -r "&& 0x" .  seems to be pretty effective modulo
> some false-positives.

Obligatory nit-picking:

	grep '&&[ 	]*0[xX][fF]' -r .

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Dave Jones]

On Thu, Nov 09, 2006 at 01:20:46AM +0300, Alexey Dobriyan wrote:
 > >  > > -	params->pci_rev = class_rev && 0xff;
 > >  > > +	params->pci_rev = class_rev & 0xff;
 > >  >
 > >  > Hi,
 > >  > any kind of automated detection on that one?
 > >
 > > grep -r "&& 0x" .  seems to be pretty effective modulo
 > > some false-positives.
 > 
 > Obligatory nit-picking:
 > 
 > 	grep '&&[ 	]*0[xX][fF]' -r .

That misses some cases. Like..

drivers/char/ipmi/ipmi_msghandler.c:                    bmc->id.device_revision && 0x80 >> 7);
drivers/char/ipmi/ipmi_msghandler.c:                    bmc->id.device_revision && 0x0F);

		Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Willy Tarreau]

On Wed, Nov 08, 2006 at 05:50:07PM -0500, Dave Jones wrote:
> On Thu, Nov 09, 2006 at 01:20:46AM +0300, Alexey Dobriyan wrote:
>  > >  > > -	params->pci_rev = class_rev && 0xff;
>  > >  > > +	params->pci_rev = class_rev & 0xff;
>  > >  >
>  > >  > Hi,
>  > >  > any kind of automated detection on that one?
>  > >
>  > > grep -r "&& 0x" .  seems to be pretty effective modulo
>  > > some false-positives.
>  > 
>  > Obligatory nit-picking:
>  > 
>  > 	grep '&&[ 	]*0[xX][fF]' -r .
> 
> That misses some cases. Like..
> 
> drivers/char/ipmi/ipmi_msghandler.c:                    bmc->id.device_revision && 0x80 >> 7);
> drivers/char/ipmi/ipmi_msghandler.c:                    bmc->id.device_revision && 0x0F);

Interesting grep. I found that cmpci, gdth, net1080 and nv_setup are affected
too in my rather old 2.6.18-rc4 tree. More importantly, I found a few ones in
2.4 that I will have to address.

Thanks guys for the good idea. Once again, it shows that pure code review
would considerably help finding such bugs.

Cheers,
Willy

Re: [PATCH] gx-suspmod: fix "&& 0xff" typo

[Horst H. von Brand]

Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >  > > -	params->pci_rev = class_rev && 0xff;
> >  > > +	params->pci_rev = class_rev & 0xff;
> >  >
> >  > Hi,
> >  > any kind of automated detection on that one?
> >
> > grep -r "&& 0x" .  seems to be pretty effective modulo
> > some false-positives.
> 
> Obligatory nit-picking:
> 
> 	grep '&&[ 	]*0[xX][fF]' -r .

Hum... could also be, e.g., "&& 1 << 3", or "&& SOME_FUNKY_CONSTANT"...

        grep -r '&&[    ]*[0-9A-Z]'

but that gives lots of noise... need to refine (or just eyeball the
output). OK, will check.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513