Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
From: Pablo Pellecchia <pablo9891@gmail.com>
To: "Valdis Klētnieks" <valdis.kletnieks@vt.edu>
Cc: kernelnewbies@kernelnewbies.org
Subject: Re: Staging/netlogic coding style issues with struct
Date: Wed, 4 Sep 2019 00:27:02 -0300
Message-ID: <CADN=kBndfw_UTaX=0TkCEKBDDAcyBorG_TfMwad=hi6qGvvZWA@mail.gmail.com> (raw)
In-Reply-To: <542004.1567506067@turing-police>

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

      reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03  4:26 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 message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADN=kBndfw_UTaX=0TkCEKBDDAcyBorG_TfMwad=hi6qGvvZWA@mail.gmail.com' \
    --to=pablo9891@gmail.com \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=valdis.kletnieks@vt.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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