* Staging/netlogic coding style issues with struct
@ 2019-09-03 4:26 Pablo Pellecchia
2019-09-03 6:50 ` Greg KH
2019-09-03 10:21 ` Valdis Klētnieks
0 siblings, 2 replies; 4+ messages in thread
From: Pablo Pellecchia @ 2019-09-03 4:26 UTC (permalink / raw)
To: kernelnewbies
[-- Attachment #1.1: Type: text/plain, Size: 429 bytes --]
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!
[-- Attachment #1.2: Type: text/html, Size: 650 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Staging/netlogic coding style issues with struct 2019-09-03 4:26 Staging/netlogic coding style issues with struct Pablo Pellecchia @ 2019-09-03 6:50 ` Greg KH 2019-09-03 10:21 ` Valdis Klētnieks 1 sibling, 0 replies; 4+ messages in thread From: Greg KH @ 2019-09-03 6:50 UTC (permalink / raw) To: Pablo Pellecchia; +Cc: kernelnewbies 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 _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Staging/netlogic coding style issues with struct 2019-09-03 4:26 Staging/netlogic coding style issues with struct Pablo Pellecchia 2019-09-03 6:50 ` Greg KH @ 2019-09-03 10:21 ` Valdis Klētnieks 2019-09-04 3:27 ` Pablo Pellecchia 1 sibling, 1 reply; 4+ messages in thread From: Valdis Klētnieks @ 2019-09-03 10:21 UTC (permalink / raw) To: Pablo Pellecchia; +Cc: kernelnewbies [-- Attachment #1.1: Type: text/plain, Size: 2437 bytes --] 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.... [-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 170 bytes --] _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Staging/netlogic coding style issues with struct 2019-09-03 10:21 ` Valdis Klētnieks @ 2019-09-04 3:27 ` Pablo Pellecchia 0 siblings, 0 replies; 4+ messages in thread From: Pablo Pellecchia @ 2019-09-04 3:27 UTC (permalink / raw) To: Valdis Klētnieks; +Cc: kernelnewbies [-- Attachment #1.1: Type: text/plain, Size: 2831 bytes --] 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.... > > > [-- Attachment #1.2: Type: text/html, Size: 3690 bytes --] [-- Attachment #2: Type: text/plain, Size: 170 bytes --] _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-04 3:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-03 4:26 Staging/netlogic coding style issues with struct Pablo Pellecchia 2019-09-03 6:50 ` Greg KH 2019-09-03 10:21 ` Valdis Klētnieks 2019-09-04 3:27 ` Pablo Pellecchia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).