[PATCH] checkkconfigsymbols.py: filter reports for tools/

[PATCH] checkkconfigsymbols.py: filter reports for tools/

[Valentin Rothberg]

Recent changes to the build system of tools suggest to filter reports
for the entire tools directory.  Various C preprocessor identifiers are
prefixed with CONFIG_ but are NOT defined in Kconfig but in Makefiles in
the tools directory.  Such identifiers are false positives for most static
analysis tools (i.e., scripts/checkkconfigsymbols.py) since the CONFIG_
prefix and the _MODULE suffix is reserved for Kconfig features in CPP
and Make syntax.

Signed-off-by: Valentin Rothberg <Valentin.Rothberg@lip6.fr>
---
 scripts/checkkconfigsymbols.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 scripts/checkkconfigsymbols.py

diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
old mode 100644
new mode 100755
index e9cc689..6445693
--- a/scripts/checkkconfigsymbols.py
+++ b/scripts/checkkconfigsymbols.py
@@ -2,7 +2,7 @@
 
 """Find Kconfig identifiers that are referenced but not defined."""
 
-# (c) 2014 Valentin Rothberg <valentinrothberg@gmail.com>
+# (c) 2014-2015 Valentin Rothberg <Valentin.Rothberg@lip6.fr>
 # (c) 2014 Stefan Hengelein <stefan.hengelein@fau.de>
 #
 # Licensed under the terms of the GNU GPL License version 2
@@ -46,8 +46,9 @@ def main():
         stdout = stdout[:-1]
 
     for gitfile in stdout.rsplit("\n"):
-        if ".git" in gitfile or "ChangeLog" in gitfile or \
-                ".log" in gitfile or os.path.isdir(gitfile):
+        if ".git" in gitfile or "ChangeLog" in gitfile or      \
+                ".log" in gitfile or os.path.isdir(gitfile) or \
+                gitfile.startswith("tools/"):
             continue
         if REGEX_FILE_KCONFIG.match(gitfile):
             kconfig_files.append(gitfile)
-- 
1.9.1

Re: [PATCH] checkkconfigsymbols.py: filter reports for tools/

[Paul Bolle]

On Wed, 2015-02-25 at 15:15 +0100, Valentin Rothberg wrote:
> @@ -46,8 +46,9 @@ def main():
>          stdout = stdout[:-1]
>  
>      for gitfile in stdout.rsplit("\n"):
> -        if ".git" in gitfile or "ChangeLog" in gitfile or \
> -                ".log" in gitfile or os.path.isdir(gitfile):
> +        if ".git" in gitfile or "ChangeLog" in gitfile or      \
> +                ".log" in gitfile or os.path.isdir(gitfile) or \
> +                gitfile.startswith("tools/"):

Perhaps just
    gitfile == "tools/perf/config/Makefile"

(but I'm unsure if that's valid python)? 

>              continue
>          if REGEX_FILE_KCONFIG.match(gitfile):
>              kconfig_files.append(gitfile)

This patch was triggered by perf changes that hit next-20150225, wasn't
it? If so, we might want to find out why the perf people need to use
their
    "$(call detected,CONFIG_EXAMPLE)"

hack. Especially because that hack is also used on existing Kconfig
symbols (I spotted X86, X86_64, AUDIT, and NUMA). And the usage of both
valid Kconfig macros and faux Kconfig macros in that hack looks odd to
me.


Paul Bolle

Re: [PATCH] checkkconfigsymbols.py: filter reports for tools/

[Valentin Rothberg]

Hi Paul,

thanks for your answer.

On Tue, Mar 3, 2015 at 1:57 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Wed, 2015-02-25 at 15:15 +0100, Valentin Rothberg wrote:
>> @@ -46,8 +46,9 @@ def main():
>>          stdout = stdout[:-1]
>>
>>      for gitfile in stdout.rsplit("\n"):
>> -        if ".git" in gitfile or "ChangeLog" in gitfile or \
>> -                ".log" in gitfile or os.path.isdir(gitfile):
>> +        if ".git" in gitfile or "ChangeLog" in gitfile or      \
>> +                ".log" in gitfile or os.path.isdir(gitfile) or \
>> +                gitfile.startswith("tools/"):
>
> Perhaps just
>     gitfile == "tools/perf/config/Makefile"
>
> (but I'm unsure if that's valid python)?
>
>>              continue
>>          if REGEX_FILE_KCONFIG.match(gitfile):
>>              kconfig_files.append(gitfile)
>
> This patch was triggered by perf changes that hit next-20150225, wasn't
> it? If so, we might want to find out why the perf people need to use

Yes, it was in next-20150225.  However, more recent changes have the
same problem.  I fear it get's worse for us : )

> their
>     "$(call detected,CONFIG_EXAMPLE)"
>
> hack. Especially because that hack is also used on existing Kconfig
> symbols (I spotted X86, X86_64, AUDIT, and NUMA). And the usage of both
> valid Kconfig macros and faux Kconfig macros in that hack looks odd to
> me.

AFAIU it's independent from Kconfig / Kbuild.  The usage of Kconfig
symbols seems completely random to me.

Ignoring tools entirely also seems a little too much, since some tools
are still Kconfig sensitive.    Hence, I vote to ignore only perf:

+                gitfile.startswith("tools/perf"):

Re: [PATCH] checkkconfigsymbols.py: filter reports for tools/

[Paul Bolle]

On Tue, 2015-03-03 at 08:55 +0100, Valentin Rothberg wrote:
> On Tue, Mar 3, 2015 at 1:57 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > On Wed, 2015-02-25 at 15:15 +0100, Valentin Rothberg wrote:
> > This patch was triggered by perf changes that hit next-20150225, wasn't
> > it? If so, we might want to find out why the perf people need to use
> 
> Yes, it was in next-20150225.  However, more recent changes have the
> same problem.  I fear it get's worse for us : )
> 
> > their
> >     "$(call detected,CONFIG_EXAMPLE)"
> >
> > hack. Especially because that hack is also used on existing Kconfig
> > symbols (I spotted X86, X86_64, AUDIT, and NUMA). And the usage of both
> > valid Kconfig macros and faux Kconfig macros in that hack looks odd to
> > me.
> 
> AFAIU it's independent from Kconfig / Kbuild.  The usage of Kconfig
> symbols seems completely random to me.

I think the entire "call detected" mechanism might as well use, say,
DETECTED_ for a prefix. That would solve our worries. Please have a look
at that hack, and see if I got that right. In that case we might as well
send a patch to do something like that and see if the perf developers
bark.

> Ignoring tools entirely also seems a little too much, since some tools
> are still Kconfig sensitive.    Hence, I vote to ignore only perf:
> 
> +                gitfile.startswith("tools/perf"):

My suggestion to only use "tools/perf/config/Makefile" was silly, and
you missed an opportunity to mock me.

tools/ is special. I'm unsure how to handle it. Assuming
checkkconfigsymbols.py will not be used by the "checker complains, so
patches must be sent" crowd, I suggest to do nothing. But it's your
script, so I defer to your decision.


Paul Bolle