* [U-Boot] [BUG] checkpatch: false positive storage class location @ 2017-07-04 19:29 Heinrich Schuchardt 2017-07-04 20:44 ` Joe Perches 0 siblings, 1 reply; 4+ messages in thread From: Heinrich Schuchardt @ 2017-07-04 19:29 UTC (permalink / raw) To: u-boot The U-Boot project uses the same scripts/checkpatch.pl as the Linux kernel. I ran upon the problem below when working on U-Boot. But I guess it should be fixed in the Linux upstream. Running checkpatch for this email produces WARNING: storage class should be at the beginning of the declaration This relates to the parameter with asmlinkage. asmlinkage is at the start of the parameter so I think this a false positive. Signed-off-by: Heinrich.Schuchardt <xypron.glpk@gmx.de> --- cmd/bootefi.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 771300ee94..4df468307c 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -147,6 +147,12 @@ static void *copy_fdt(void *fdt) return new_fdt; } +static ulong efi_do_enter(void *image_handle, + struct efi_system_table *st, asmlinkage ulong (*entry)( + void *image_handle, struct efi_system_table *st)) +{ + return 0; +} + /* end */ -- 2.11.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [BUG] checkpatch: false positive storage class location 2017-07-04 19:29 [U-Boot] [BUG] checkpatch: false positive storage class location Heinrich Schuchardt @ 2017-07-04 20:44 ` Joe Perches 2017-07-04 21:08 ` Heinrich Schuchardt 0 siblings, 1 reply; 4+ messages in thread From: Joe Perches @ 2017-07-04 20:44 UTC (permalink / raw) To: u-boot On Tue, 2017-07-04 at 21:29 +0200, Heinrich Schuchardt wrote: > The U-Boot project uses the same scripts/checkpatch.pl as the Linux > kernel. I ran upon the problem below when working on U-Boot. But I > guess it should be fixed in the Linux upstream. > > Running checkpatch for this email produces > WARNING: storage class should be at the beginning of the declaration > > This relates to the parameter with asmlinkage. > > asmlinkage is at the start of the parameter so I think this a false > positive. > > Signed-off-by: Heinrich.Schuchardt <xypron.glpk@gmx.de> > --- > > cmd/bootefi.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 771300ee94..4df468307c 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -147,6 +147,12 @@ static void *copy_fdt(void *fdt) > return new_fdt; > } > > +static ulong efi_do_enter(void *image_handle, > + struct efi_system_table *st, asmlinkage ulong (*entry)( > + void *image_handle, struct efi_system_table *st)) > +{ > + return 0; > +} > + > /* > end > */ Perhaps this? ------------------------- Allow storage class after comma for function pointers. Miscellanea: o Add missing semicolon after WARN statement --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 43171ed88115..c7490ab48ce1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5577,9 +5577,10 @@ sub process { } # Check that the storage class is at the beginning of a declaration - if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + if ($line =~ /\b$Storage\b/ && + $line !~ /^.\s*(?:.*,\s*)?$Storage\b/) { WARN("STORAGE_CLASS", - "storage class should be at the beginning of the declaration\n" . $herecurr) + "storage class should be at the beginning of the declaration\n" . $herecurr); } # check the location of the inline attribute, that it is between ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [BUG] checkpatch: false positive storage class location 2017-07-04 20:44 ` Joe Perches @ 2017-07-04 21:08 ` Heinrich Schuchardt 2017-07-05 12:01 ` Joe Perches 0 siblings, 1 reply; 4+ messages in thread From: Heinrich Schuchardt @ 2017-07-04 21:08 UTC (permalink / raw) To: u-boot On 07/04/2017 10:44 PM, Joe Perches wrote: > On Tue, 2017-07-04 at 21:29 +0200, Heinrich Schuchardt wrote: >> The U-Boot project uses the same scripts/checkpatch.pl as the Linux >> kernel. I ran upon the problem below when working on U-Boot. But I >> guess it should be fixed in the Linux upstream. >> >> Running checkpatch for this email produces >> WARNING: storage class should be at the beginning of the declaration >> >> This relates to the parameter with asmlinkage. >> >> asmlinkage is at the start of the parameter so I think this a false >> positive. >> >> Signed-off-by: Heinrich.Schuchardt <xypron.glpk@gmx.de> >> --- >> >> cmd/bootefi.c | 22 +++++++++++++++++----- >> 1 file changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index 771300ee94..4df468307c 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -147,6 +147,12 @@ static void *copy_fdt(void *fdt) >> return new_fdt; >> } >> >> +static ulong efi_do_enter(void *image_handle, >> + struct efi_system_table *st, asmlinkage ulong (*entry)( >> + void *image_handle, struct efi_system_table *st)) >> +{ >> + return 0; >> +} >> + >> /* >> end >> */ > > Perhaps this? > ------------------------- > > Allow storage class after comma for function pointers. > > Miscellanea: > > o Add missing semicolon after WARN statement > --- > scripts/checkpatch.pl | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 43171ed88115..c7490ab48ce1 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -5577,9 +5577,10 @@ sub process { > } > > # Check that the storage class is at the beginning of a declaration > - if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { > + if ($line =~ /\b$Storage\b/ && > + $line !~ /^.\s*(?:.*,\s*)?$Storage\b/) { > WARN("STORAGE_CLASS", > - "storage class should be at the beginning of the declaration\n" . $herecurr) > + "storage class should be at the beginning of the declaration\n" . $herecurr); > } > > # check the location of the inline attribute, that it is between > Thank you. This works for all cases but +static ulong efi_do_enter(ulong asmlinkage (*entry)(void *image_handle, + struct efi_system_table *st)) +{ + return 0; +} + Here I get WARNING: space prohibited between function name and open parenthesis '(' where I would have expected WARNING: storage class should be at the beginning of the declaration Best regards Heinrich Schuchardt ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [BUG] checkpatch: false positive storage class location 2017-07-04 21:08 ` Heinrich Schuchardt @ 2017-07-05 12:01 ` Joe Perches 0 siblings, 0 replies; 4+ messages in thread From: Joe Perches @ 2017-07-05 12:01 UTC (permalink / raw) To: u-boot On Tue, 2017-07-04 at 23:08 +0200, Heinrich Schuchardt wrote: > On 07/04/2017 10:44 PM, Joe Perches wrote: > > On Tue, 2017-07-04 at 21:29 +0200, Heinrich Schuchardt wrote: > > > The U-Boot project uses the same scripts/checkpatch.pl as the Linux > > > kernel. I ran upon the problem below when working on U-Boot. But I > > > guess it should be fixed in the Linux upstream. > > > > > > Running checkpatch for this email produces > > > WARNING: storage class should be at the beginning of the declaration > > > > > > This relates to the parameter with asmlinkage. > > > > > > asmlinkage is at the start of the parameter so I think this a false > > > positive. > > > > > > Signed-off-by: Heinrich.Schuchardt <xypron.glpk@gmx.de> > > > --- > > > > > > cmd/bootefi.c | 22 +++++++++++++++++----- > > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > > > index 771300ee94..4df468307c 100644 > > > --- a/cmd/bootefi.c > > > +++ b/cmd/bootefi.c > > > @@ -147,6 +147,12 @@ static void *copy_fdt(void *fdt) > > > return new_fdt; > > > } > > > > > > +static ulong efi_do_enter(void *image_handle, > > > + struct efi_system_table *st, asmlinkage ulong (*entry)( > > > + void *image_handle, struct efi_system_table *st)) > > > +{ > > > + return 0; > > > +} > > > + > > > /* > > > end > > > */ > > > > Perhaps this? > > ------------------------- > > > > Allow storage class after comma for function pointers. > > > > Miscellanea: > > > > o Add missing semicolon after WARN statement > > --- > > scripts/checkpatch.pl | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 43171ed88115..c7490ab48ce1 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -5577,9 +5577,10 @@ sub process { > > } > > > > # Check that the storage class is at the beginning of a declaration > > - if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { > > + if ($line =~ /\b$Storage\b/ && > > + $line !~ /^.\s*(?:.*,\s*)?$Storage\b/) { > > WARN("STORAGE_CLASS", > > - "storage class should be at the beginning of the declaration\n" . $herecurr) > > + "storage class should be at the beginning of the declaration\n" . $herecurr); > > } > > > > # check the location of the inline attribute, that it is between > > > > Thank you. This works for all cases but > > +static ulong efi_do_enter(ulong asmlinkage (*entry)(void *image_handle, > + struct efi_system_table *st)) > +{ > + return 0; > +} > + > > Here I get > WARNING: space prohibited between function name and open parenthesis '(' > where I would have expected > WARNING: storage class should be at the beginning of the declaration checkpatch is and always will be a stupid brainless little script based on a collection of regexes. It won't get everything correct. Your example has static as the first entry on the line. static is a "$Storage" so it is at the beginning of the declaration. This particular declaration has a function pointer as one of its arguments. I do wonder if asmlinkage is even appropriate as part of that function pointer argument. Anyway, maybe this: --- scripts/checkpatch.pl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 43171ed88115..21bb6814f8bb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5576,10 +5576,18 @@ sub process { "architecture specific defines should be avoided\n" . $herecurr); } +# check that the storage class is not after a type + if ($line =~ /\b($Type)\s+($Storage)\b/) { + WARN("STORAGE_CLASS", + "storage class '$2' should be located before type '$1'\n" . $herecurr); + } # Check that the storage class is at the beginning of a declaration - if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + if ($line =~ /\b$Storage\b/ && + $line !~ /^.\s*$Storage/ && + $line =~ /^.\s*(.+?)\$Storage\s/ && + $1 !~ /[\,\)]\s*$/) { WARN("STORAGE_CLASS", - "storage class should be at the beginning of the declaration\n" . $herecurr) + "storage class should be at the beginning of the declaration\n" . $herecurr); } # check the location of the inline attribute, that it is between ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-05 12:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-04 19:29 [U-Boot] [BUG] checkpatch: false positive storage class location Heinrich Schuchardt 2017-07-04 20:44 ` Joe Perches 2017-07-04 21:08 ` Heinrich Schuchardt 2017-07-05 12:01 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox