Staging/netlogic coding style issues with struct
Greetings, I'm fixing some issues on the staging/netlogic directory and I see that checkpatch.pl is throwing the following warnings on some files: *WARNING: struct should normally be const#9: FILE: platform_net.h:9:+struct xlr_net_data {* A similar issue is reported when we declare a variable of type struct <something>, but in this case warning is reported on the struct definition itself. How can we fix this? Thanks!
On Tue, Sep 03, 2019 at 01:26:17AM -0300, Pablo Pellecchia wrote:
Greetings,
I'm fixing some issues on the staging/netlogic directory and I see that checkpatch.pl is throwing the following warnings on some files:
*WARNING: struct should normally be const#9: FILE: platform_net.h:9:+struct xlr_net_data {*
A similar issue is reported when we declare a variable of type struct <something>, but in this case warning is reported on the struct definition itself.
How can we fix this?
You mark the structures const in the correct places as needed. Sometimes it is not needed, checkpatch is just a perl script and tries to do the best it can. Please read up on how C uses const if you are unfamiliar with what that means. thanks, greg k-h
On Tue, 03 Sep 2019 01:26:17 -0300, Pablo Pellecchia said:
*WARNING: struct should normally be const#9: FILE: platform_net.h:9:+struct xlr_net_data {*
A similar issue is reported when we declare a variable of type struct <something>, but in this case warning is reported on the struct definition itself.
How can we fix this?
And in today's "How to debug checkpatch" lesson.. :) First, figure out if checkpatch is in fact correct. It' just a Perl script, and has no real idea of what the code is. And double-checking, there's very few 'const struct' declarations in include/linux/*.h. So what's going on? Good question. Actually looking at checkpatch.pl, we find: # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); } and $const_structs is initialized from scripts/const_structs.checkpatch And that tells us 2 things: First, this should only be triggering for structures that are listed in that file, and the message *should* say something like 'struct foo should normally be const', with $1 filling in the struct name. So why is $1 not showing up? Damned good question. And the file checks just fine for me. [/usr/src/linux-next]2 scripts/checkpatch.pl -f drivers/staging/netlogic/platform_net.h total: 0 errors, 0 warnings, 0 checks, 21 lines checked drivers/staging/netlogic/platform_net.h has no obvious style problems and is ready for submission. Bingo! This is what happens if the permissions on the file are messed up and it can't read the file: [/usr/src/linux-next] scripts/checkpatch.pl -f drivers/staging/netlogic/platform_net.h No structs that should be const will be found - file '/usr/src/linux-next/scripts/const_structs.checkpatch': Permission denied WARNING: struct should normally be const #9: FILE: drivers/staging/netlogic/platform_net.h:9: +struct xlr_net_data { So... you probably need to check the permissions, or if the file is missing from your tree or empty or something. The version in my tree is 64 lines long. Meanwhile, I'm going to go cook up a patch for this....
Yeap! That solved the problem. Seems I was not using correctly the checkpatch.pl file. My bad. Now everything seems to be working fine. Thanks for the help! El mar., 3 sept. 2019 a las 7:21, Valdis Klētnieks (<valdis.kletnieks@vt.edu>) escribió:
On Tue, 03 Sep 2019 01:26:17 -0300, Pablo Pellecchia said:
*WARNING: struct should normally be const#9: FILE: platform_net.h:9:+struct xlr_net_data {*
A similar issue is reported when we declare a variable of type struct <something>, but in this case warning is reported on the struct definition itself.
How can we fix this?
And in today's "How to debug checkpatch" lesson.. :)
First, figure out if checkpatch is in fact correct. It' just a Perl script, and has no real idea of what the code is.
And double-checking, there's very few 'const struct' declarations in include/linux/*.h.
So what's going on? Good question. Actually looking at checkpatch.pl, we find:
# check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); }
and $const_structs is initialized from scripts/const_structs.checkpatch And that tells us 2 things: First, this should only be triggering for structures that are listed in that file, and the message *should* say something like 'struct foo should normally be const', with $1 filling in the struct name.
So why is $1 not showing up? Damned good question. And the file checks just fine for me.
[/usr/src/linux-next]2 scripts/checkpatch.pl -f drivers/staging/netlogic/platform_net.h total: 0 errors, 0 warnings, 0 checks, 21 lines checked
drivers/staging/netlogic/platform_net.h has no obvious style problems and is ready for submission.
Bingo! This is what happens if the permissions on the file are messed up and it can't read the file:
[/usr/src/linux-next] scripts/checkpatch.pl -f drivers/staging/netlogic/platform_net.h No structs that should be const will be found - file '/usr/src/linux-next/scripts/const_structs.checkpatch': Permission denied WARNING: struct should normally be const #9: FILE: drivers/staging/netlogic/platform_net.h:9: +struct xlr_net_data {
So... you probably need to check the permissions, or if the file is missing from your tree or empty or something. The version in my tree is 64 lines long.
Meanwhile, I'm going to go cook up a patch for this....
participants (3)
-
Greg KH -
Pablo Pellecchia -
Valdis Klētnieks