Staging/netlogic coding style issues with struct

Staging/netlogic coding style issues with struct

[Pablo Pellecchia]

[-- 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

Re: Staging/netlogic coding style issues with struct

[Greg KH]

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

Re: Staging/netlogic coding style issues with struct

[Valdis Klētnieks]

[-- 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

Re: Staging/netlogic coding style issues with struct

[Pablo Pellecchia]

[-- 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