Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
* 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 --]

<div dir="ltr"><div>Greetings,</div><div><br></div><div>I&#39;m fixing some issues on the staging/netlogic directory and I see that <a href="http://checkpatch.pl">checkpatch.pl</a> is throwing the following warnings on some files:</div><div><i><br></i></div><div><i>WARNING: struct  should normally be const<br>#9: FILE: platform_net.h:9:<br>+struct xlr_net_data {</i></div><div><br></div><div>A similar issue is reported when we declare a variable of type struct &lt;something&gt;, but in this case warning is reported on the struct definition itself.</div><div><br></div><div>How can we fix this?</div><div><br></div><div>Thanks!<br></div></div>

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

<div dir="ltr"><div>Yeap! That solved the problem. Seems I was not using correctly the <a href="http://checkpatch.pl">checkpatch.pl</a> file. My bad.</div><div><br></div><div>Now everything seems to be working fine. </div><div><br></div><div>Thanks for the help!<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">El mar., 3 sept. 2019 a las 7:21, Valdis Klētnieks (&lt;<a href="mailto:valdis.kletnieks@vt.edu">valdis.kletnieks@vt.edu</a>&gt;) escribió:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 03 Sep 2019 01:26:17 -0300, Pablo Pellecchia said:<br>
<br>
&gt; *WARNING: struct  should normally be const#9: FILE:<br>
&gt; platform_net.h:9:+struct xlr_net_data {*<br>
&gt;<br>
&gt; A similar issue is reported when we declare a variable of type struct<br>
&gt; &lt;something&gt;, but in this case warning is reported on the struct definition<br>
&gt; itself.<br>
&gt;<br>
&gt; How can we fix this?<br>
<br>
And in today&#39;s &quot;How to debug checkpatch&quot; lesson.. :)<br>
<br>
First, figure out if checkpatch is in fact correct. It&#39; just a Perl script,<br>
and has no real idea of what the code is.<br>
<br>
And double-checking, there&#39;s very few &#39;const struct&#39; declarations in<br>
include/linux/*.h.<br>
<br>
So what&#39;s going on?  Good question. Actually looking at <a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a>,<br>
we find:<br>
<br>
# check for various structs that are normally const (ops, kgdb, device_tree)<br>
# and avoid what seem like struct definitions &#39;struct foo {&#39;<br>
                if ($line !~ /\bconst\b/ &amp;&amp;<br>
                    $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {<br>
                        WARN(&quot;CONST_STRUCT&quot;,<br>
                             &quot;struct $1 should normally be const\n&quot; . $herecurr);<br>
                }<br>
<br>
and $const_structs is initialized from scripts/const_structs.checkpatch <br>
And that tells us 2 things:  First, this should only be triggering for structures<br>
that are listed in that file, and the message *should* say something<br>
like &#39;struct foo should normally be const&#39;, with $1 filling in the struct name.<br>
<br>
So why is $1 not showing up? Damned good question.  And the file<br>
checks just fine for me.<br>
<br>
[/usr/src/linux-next]2 scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a> -f drivers/staging/netlogic/platform_net.h<br>
total: 0 errors, 0 warnings, 0 checks, 21 lines checked<br>
<br>
drivers/staging/netlogic/platform_net.h has no obvious style problems and is ready for submission.<br>
<br>
Bingo!  This is what happens if the permissions on the file are messed up<br>
and it can&#39;t read the file:<br>
<br>
[/usr/src/linux-next] scripts/<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a> -f drivers/staging/netlogic/platform_net.h<br>
No structs that should be const will be found - file &#39;/usr/src/linux-next/scripts/const_structs.checkpatch&#39;: Permission denied<br>
WARNING: struct  should normally be const<br>
#9: FILE: drivers/staging/netlogic/platform_net.h:9:<br>
+struct xlr_net_data {<br>
<br>
So... you probably need to check the permissions, or if the file is missing<br>
from your tree or empty or something. The version in my tree is 64 lines long.<br>
<br>
Meanwhile, I&#39;m going to go cook up a patch for this....<br>
<br>
<br>
</blockquote></div>

[-- 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, back to index

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

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org kernelnewbies@archiver.kernel.org
	public-inbox-index kernelnewbies


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/ public-inbox