Re: [PATCH v2] checkpatch.pl: Warn on duplicate sysctl local variable
On Fri, 2019-05-31 at 10:44 +0200, Matteo Croce wrote: > On May 31, 2019 5:06:58 AM GMT+02:00, Joe Perches wrote: > > On Fri, 2019-05-31 at 03:12 +0200, Matteo Croce wrote: > > > + > > > +# check for sysctl duplicate constants > > > + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max|max_int)\b/) > > > { > > > > why max_int, there isn't a single use of it in the kernel ? > > Because you can never know how a local variabile will be called. > I wanted to add intmax and maxint too, bit it seemed too much. I think that checkpatch should not speculate about things like this.
Re: [PATCH v2] checkpatch.pl: Warn on duplicate sysctl local variable
On May 31, 2019 5:06:58 AM GMT+02:00, Joe Perches wrote: > On Fri, 2019-05-31 at 03:12 +0200, Matteo Croce wrote: > > Commit 6a33853c5773 ("proc/sysctl: add shared variables for range > check") > > adds some shared const variables to be used instead of a local copy > in > > each source file. > > Warn when a chunk duplicates one of these values in a ctl_table > struct: > > > > $ scripts/checkpatch.pl 0001-test-commit.patch > > WARNING: duplicated sysctl range checking value 'zero', consider > using the shared one in include/linux/sysctl.h > > #27: FILE: arch/arm/kernel/isa.c:48: > > + .extra1 = , > > > > WARNING: duplicated sysctl range checking value 'int_max', > consider using the shared one in include/linux/sysctl.h > > #28: FILE: arch/arm/kernel/isa.c:49: > > + .extra2 = _max, > > > > total: 0 errors, 2 warnings, 14 lines checked > > > > Signed-off-by: Matteo Croce > > --- > > scripts/checkpatch.pl | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 342c7c781ba5..629c31435487 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -6639,6 +6639,12 @@ sub process { > > "unknown module license " . > > $extracted_string . "\n" . > $herecurr); > > } > > } > > + > > +# check for sysctl duplicate constants > > + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max|max_int)\b/) > > { > > why max_int, there isn't a single use of it in the kernel ? Because you can never know how a local variabile will be called. I wanted to add intmax and maxint too, bit it seemed too much. Bye, -- Matteo Croce per aspera ad upstream
Re: [PATCH v2] checkpatch.pl: Warn on duplicate sysctl local variable
On Fri, 2019-05-31 at 03:12 +0200, Matteo Croce wrote: > Commit 6a33853c5773 ("proc/sysctl: add shared variables for range check") > adds some shared const variables to be used instead of a local copy in > each source file. > Warn when a chunk duplicates one of these values in a ctl_table struct: > > $ scripts/checkpatch.pl 0001-test-commit.patch > WARNING: duplicated sysctl range checking value 'zero', consider using > the shared one in include/linux/sysctl.h > #27: FILE: arch/arm/kernel/isa.c:48: > + .extra1 = , > > WARNING: duplicated sysctl range checking value 'int_max', consider using > the shared one in include/linux/sysctl.h > #28: FILE: arch/arm/kernel/isa.c:49: > + .extra2 = _max, > > total: 0 errors, 2 warnings, 14 lines checked > > Signed-off-by: Matteo Croce > --- > scripts/checkpatch.pl | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 342c7c781ba5..629c31435487 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -6639,6 +6639,12 @@ sub process { >"unknown module license " . > $extracted_string . "\n" . $herecurr); > } > } > + > +# check for sysctl duplicate constants > + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max|max_int)\b/) > { why max_int, there isn't a single use of it in the kernel ?
Re: [PATCH v2] checkpatch.pl: Warn on duplicate sysctl local variable
On Fri, May 31, 2019 at 03:12:27AM +0200, Matteo Croce wrote: > Commit 6a33853c5773 ("proc/sysctl: add shared variables for range check") > adds some shared const variables to be used instead of a local copy in > each source file. > Warn when a chunk duplicates one of these values in a ctl_table struct: > > $ scripts/checkpatch.pl 0001-test-commit.patch > WARNING: duplicated sysctl range checking value 'zero', consider using > the shared one in include/linux/sysctl.h > #27: FILE: arch/arm/kernel/isa.c:48: > + .extra1 = , > > WARNING: duplicated sysctl range checking value 'int_max', consider using > the shared one in include/linux/sysctl.h > #28: FILE: arch/arm/kernel/isa.c:49: > + .extra2 = _max, > > total: 0 errors, 2 warnings, 14 lines checked > > Signed-off-by: Matteo Croce Reviewed-by: Kees Cook -Kees > --- > scripts/checkpatch.pl | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 342c7c781ba5..629c31435487 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -6639,6 +6639,12 @@ sub process { >"unknown module license " . > $extracted_string . "\n" . $herecurr); > } > } > + > +# check for sysctl duplicate constants > + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max|max_int)\b/) > { > + WARN("DUPLICATED_SYSCTL_CONST", > + "duplicated sysctl range checking value '$1', > consider using the shared one in include/linux/sysctl.h\n" . $herecurr); > + } > } > > # If we have no input at all, then there is nothing to report on > -- > 2.21.0 > -- Kees Cook
[PATCH v2] checkpatch.pl: Warn on duplicate sysctl local variable
Commit 6a33853c5773 ("proc/sysctl: add shared variables for range check") adds some shared const variables to be used instead of a local copy in each source file. Warn when a chunk duplicates one of these values in a ctl_table struct: $ scripts/checkpatch.pl 0001-test-commit.patch WARNING: duplicated sysctl range checking value 'zero', consider using the shared one in include/linux/sysctl.h #27: FILE: arch/arm/kernel/isa.c:48: + .extra1 = , WARNING: duplicated sysctl range checking value 'int_max', consider using the shared one in include/linux/sysctl.h #28: FILE: arch/arm/kernel/isa.c:49: + .extra2 = _max, total: 0 errors, 2 warnings, 14 lines checked Signed-off-by: Matteo Croce --- scripts/checkpatch.pl | 6 ++ 1 file changed, 6 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 342c7c781ba5..629c31435487 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6639,6 +6639,12 @@ sub process { "unknown module license " . $extracted_string . "\n" . $herecurr); } } + +# check for sysctl duplicate constants + if ($line =~ /\.extra[12]\s*=\s*&(zero|one|int_max|max_int)\b/) { + WARN("DUPLICATED_SYSCTL_CONST", + "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); + } } # If we have no input at all, then there is nothing to report on -- 2.21.0