Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
Josh Boyer wrote: On Wed, 23 Jan 2008 23:37:33 +0100 Roel Kluin <[EMAIL PROTECTED]> wrote: logical/bitand typo Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val && 0x1) + if (val & 0x1) cs++; val = val >> 1; } Hm, good catch. Yes, thanks! Sequoia and Rainier are currently the only boards using denali controller and have both chipselects enabled (val = 0x3). In this case the problem doesn't show up and we still get the right cs value. Thanks, Valentine. Stefan, have you had problems with this code at all? josh ___ Linuxppc-dev mailing list [EMAIL PROTECTED] https://ozlabs.org/mailman/listinfo/linuxppc-dev -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
>>> On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: - if (val && 0x1) + if (val & 0x1) >> Joe Perches wrote: >>> I think this pattern should be added to checkpatch >>> + if ($line =~ /\&\&\s*0[xX]/) { > On Thu, Jan 24, 2008 at 01:18:48AM +0100, Roel Kluin wrote: >> I agree but there will be false positives, i'd propose to change that to >> +if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)| >> +(?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) { Andy Whitcroft wrote: > That one doesn't even match the original example. Seems to be missing > some number matching. The concept seems sound though. Basically > looking for numbers which are definatly adjacent to a boolean or a brace > on both sides. You are right. here's a version that does work. It works the same as git-grep -E "((\(|&&|\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(&&|\|\|)|(&&|\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(\)|&&|\|\|))" --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..62276f7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,11 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /((\(|&&|\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(&&|\|\|)|(&&|\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(\)|&&|\|\|))/) { + WARN("boolean test with hexadecimal, perhaps just \'&\' or \'|\'?\n" . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Thu, Jan 24, 2008 at 01:18:48AM +0100, Roel Kluin wrote: > Joe Perches wrote: > > On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: > >> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > >> --- > >> diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c > >> index ebf9e21..dcfb459 100644 > >> --- a/arch/powerpc/boot/4xx.c > >> +++ b/arch/powerpc/boot/4xx.c > >> @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) > >>val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); > >>cs = 0; > >>while (val) { > >> - if (val && 0x1) > >> + if (val & 0x1) > >>cs++; > >>val = val >> 1; > > > > I think this pattern should be added to checkpatch > > I agree but... > > > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 579f50f..147e573 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -1337,6 +1337,11 @@ sub process { > > } > > } > > > > +# Check for bitwise tests written as boolean > > + if ($line =~ /\&\&\s*0[xX]/) { > > + WARN("boolean test with hexadecimal, perhaps just 1 > > \&?\n" . $herecurr); > > + } > > + > > when you use git-grep -n "\(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)", > (with s="[[:space:]]*") there will be false positives like > > if (relocation < -0x2 || 0x1fffc < relocation) > if (ARCH_SUN4C_SUN4 && addr < 0xe000 && 0x2000 - len < addr) { > (and more) > > However, if you search with > git-grep -n " \(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\()\|&&\|||\)" > you won't get these false positives, but you do get the one I fixed. > > Also there is the same logic flipped (though it did not give matches at > this time): > git-grep -n " \(&&\|||\|(\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\(&&\|||\)" > > so i'd propose to change that to > -- > Catch boolean tests with hexadecimals, based on suggestion by Joe Perches > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > --- > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 579f50f..8ac7c7b 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1337,6 +1337,12 @@ sub process { > } > } > > +# Check for bitwise tests written as boolean > + if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)| > + (?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) { > + WARN("boolean test with hexadecimal, perhaps just 1 \& > or \|?\n" . $herecurr); > + } > + > # if and else should not have general statements after it > if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && > $1 !~ /^\s*(?:\sif|{|\\|$)/) { That one doesn't even match the original example. Seems to be missing some number matching. The concept seems sound though. Basically looking for numbers which are definatly adjacent to a boolean or a brace on both sides. /me goes play with some examples to see if the falsies are better than the reports. -apw -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
Josh Boyer wrote: On Wed, 23 Jan 2008 23:37:33 +0100 Roel Kluin [EMAIL PROTECTED] wrote: logical/bitand typo Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val 0x1) + if (val 0x1) cs++; val = val 1; } Hm, good catch. Yes, thanks! Sequoia and Rainier are currently the only boards using denali controller and have both chipselects enabled (val = 0x3). In this case the problem doesn't show up and we still get the right cs value. Thanks, Valentine. Stefan, have you had problems with this code at all? josh ___ Linuxppc-dev mailing list [EMAIL PROTECTED] https://ozlabs.org/mailman/listinfo/linuxppc-dev -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: - if (val 0x1) + if (val 0x1) Joe Perches wrote: I think this pattern should be added to checkpatch + if ($line =~ /\\\s*0[xX]/) { On Thu, Jan 24, 2008 at 01:18:48AM +0100, Roel Kluin wrote: I agree but there will be false positives, i'd propose to change that to +if ($line =~ /(?:(?:\(|\\|\|\|)\s*0[xX]\s*(?:|\|\|)| +(?:\\|\|\|)\s*0[xX]\s*(?:\)||\|\|))/) { Andy Whitcroft wrote: That one doesn't even match the original example. Seems to be missing some number matching. The concept seems sound though. Basically looking for numbers which are definatly adjacent to a boolean or a brace on both sides. You are right. here's a version that does work. It works the same as git-grep -E ((\(||\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(|\|\|)|(|\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(\)||\|\|)) --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..62276f7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,11 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /((\(||\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(|\|\|)|(|\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(\)||\|\|))/) { + WARN(boolean test with hexadecimal, perhaps just \'\' or \'|\'?\n . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Thursday 24 January 2008, Josh Boyer wrote: > On Wed, 23 Jan 2008 23:37:33 +0100 > > Roel Kluin <[EMAIL PROTECTED]> wrote: > > logical/bitand typo > > > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > > --- > > diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c > > index ebf9e21..dcfb459 100644 > > --- a/arch/powerpc/boot/4xx.c > > +++ b/arch/powerpc/boot/4xx.c > > @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) > > val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); > > cs = 0; > > while (val) { > > - if (val && 0x1) > > + if (val & 0x1) > > cs++; > > val = val >> 1; > > } > > Hm, good catch. > > Stefan, have you had problems with this code at all? I'm not using the bootwrapper on a 4xx with Denali core. Best regards, Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Thursday 24 January 2008, Josh Boyer wrote: On Wed, 23 Jan 2008 23:37:33 +0100 Roel Kluin [EMAIL PROTECTED] wrote: logical/bitand typo Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val 0x1) + if (val 0x1) cs++; val = val 1; } Hm, good catch. Stefan, have you had problems with this code at all? I'm not using the bootwrapper on a 4xx with Denali core. Best regards, Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Wed, 23 Jan 2008 23:37:33 +0100 Roel Kluin <[EMAIL PROTECTED]> wrote: > logical/bitand typo > > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > --- > diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c > index ebf9e21..dcfb459 100644 > --- a/arch/powerpc/boot/4xx.c > +++ b/arch/powerpc/boot/4xx.c > @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) > val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); > cs = 0; > while (val) { > - if (val && 0x1) > + if (val & 0x1) > cs++; > val = val >> 1; > } Hm, good catch. Stefan, have you had problems with this code at all? josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Thu, 2008-01-24 at 01:18 +0100, Roel Kluin wrote: > when you use git-grep -n "\(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)", > (with s="[[:space:]]*") there will be false positives [] > so i'd propose to change that to > +# Check for bitwise tests written as boolean > + if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)| > + (?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) { > + WARN("boolean test with hexadecimal, perhaps just 1 \& > or \|?\n" . $herecurr); > + } > + All 13 false positives are "hex_constant logical_test variable". I think that Linus would say that they are all poor style. Still, fine by me. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
Joe Perches wrote: > On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: >> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> >> --- >> diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c >> index ebf9e21..dcfb459 100644 >> --- a/arch/powerpc/boot/4xx.c >> +++ b/arch/powerpc/boot/4xx.c >> @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) >> val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); >> cs = 0; >> while (val) { >> -if (val && 0x1) >> +if (val & 0x1) >> cs++; >> val = val >> 1; > > I think this pattern should be added to checkpatch I agree but... > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 579f50f..147e573 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1337,6 +1337,11 @@ sub process { > } > } > > +# Check for bitwise tests written as boolean > + if ($line =~ /\&\&\s*0[xX]/) { > + WARN("boolean test with hexadecimal, perhaps just 1 > \&?\n" . $herecurr); > + } > + when you use git-grep -n "\(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)", (with s="[[:space:]]*") there will be false positives like if (relocation < -0x2 || 0x1fffc < relocation) if (ARCH_SUN4C_SUN4 && addr < 0xe000 && 0x2000 - len < addr) { (and more) However, if you search with git-grep -n " \(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\()\|&&\|||\)" you won't get these false positives, but you do get the one I fixed. Also there is the same logic flipped (though it did not give matches at this time): git-grep -n " \(&&\|||\|(\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\(&&\|||\)" so i'd propose to change that to -- Catch boolean tests with hexadecimals, based on suggestion by Joe Perches Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..8ac7c7b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,12 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)| + (?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) { + WARN("boolean test with hexadecimal, perhaps just 1 \& or \|?\n" . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> > --- > diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c > index ebf9e21..dcfb459 100644 > --- a/arch/powerpc/boot/4xx.c > +++ b/arch/powerpc/boot/4xx.c > @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) > val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); > cs = 0; > while (val) { > - if (val && 0x1) > + if (val & 0x1) > cs++; > val = val >> 1; I think this pattern should be added to checkpatch Signed-off-by: Joe Perches <[EMAIL PROTECTED]> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..147e573 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,11 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /\&\&\s*0[xX]/) { + WARN("boolean test with hexadecimal, perhaps just 1 \&?\n" . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
logical/bitand typo Signed-off-by: Roel Kluin <[EMAIL PROTECTED]> --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val && 0x1) + if (val & 0x1) cs++; val = val >> 1; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
logical/bitand typo Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val 0x1) + if (val 0x1) cs++; val = val 1; } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val 0x1) + if (val 0x1) cs++; val = val 1; I think this pattern should be added to checkpatch Signed-off-by: Joe Perches [EMAIL PROTECTED] diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..147e573 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,11 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /\\\s*0[xX]/) { + WARN(boolean test with hexadecimal, perhaps just 1 \?\n . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
Joe Perches wrote: On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote: Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { -if (val 0x1) +if (val 0x1) cs++; val = val 1; I think this pattern should be added to checkpatch I agree but... Signed-off-by: Joe Perches [EMAIL PROTECTED] diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..147e573 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,11 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /\\\s*0[xX]/) { + WARN(boolean test with hexadecimal, perhaps just 1 \?\n . $herecurr); + } + when you use git-grep -n \(\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\), (with s=[[:space:]]*) there will be false positives like if (relocation -0x2 || 0x1fffc relocation) if (ARCH_SUN4C_SUN4 addr 0xe000 0x2000 - len addr) { (and more) However, if you search with git-grep -n \(\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\()\|\|||\) you won't get these false positives, but you do get the one I fixed. Also there is the same logic flipped (though it did not give matches at this time): git-grep -n \(\|||\|(\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\(\|||\) so i'd propose to change that to -- Catch boolean tests with hexadecimals, based on suggestion by Joe Perches Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..8ac7c7b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1337,6 +1337,12 @@ sub process { } } +# Check for bitwise tests written as boolean + if ($line =~ /(?:(?:\(|\\|\|\|)\s*0[xX]\s*(?:|\|\|)| + (?:\\|\|\|)\s*0[xX]\s*(?:\)||\|\|))/) { + WARN(boolean test with hexadecimal, perhaps just 1 \ or \|?\n . $herecurr); + } + # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ $1 !~ /^\s*(?:\sif|{|\\|$)/) { -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Thu, 2008-01-24 at 01:18 +0100, Roel Kluin wrote: when you use git-grep -n \(\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\), (with s=[[:space:]]*) there will be false positives [] so i'd propose to change that to +# Check for bitwise tests written as boolean + if ($line =~ /(?:(?:\(|\\|\|\|)\s*0[xX]\s*(?:|\|\|)| + (?:\\|\|\|)\s*0[xX]\s*(?:\)||\|\|))/) { + WARN(boolean test with hexadecimal, perhaps just 1 \ or \|?\n . $herecurr); + } + All 13 false positives are hex_constant logical_test variable. I think that Linus would say that they are all poor style. Still, fine by me. cheers, Joe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c
On Wed, 23 Jan 2008 23:37:33 +0100 Roel Kluin [EMAIL PROTECTED] wrote: logical/bitand typo Signed-off-by: Roel Kluin [EMAIL PROTECTED] --- diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c index ebf9e21..dcfb459 100644 --- a/arch/powerpc/boot/4xx.c +++ b/arch/powerpc/boot/4xx.c @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void) val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT); cs = 0; while (val) { - if (val 0x1) + if (val 0x1) cs++; val = val 1; } Hm, good catch. Stefan, have you had problems with this code at all? josh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/