[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2bba4c0a-8639-1d3a-5dd5-8e2576f6ab77@gmail.com>
Date: Sun, 29 Aug 2021 16:17:59 +0300
From: Ariel Marcovitch <arielmarcovitch@...il.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Michal Marek <michal.lkml@...kovi.net>,
Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Valentin Rothberg <valentinrothberg@...il.com>
Subject: Re: [PATCH 2/3] checkkconfigsymbols.py: Fix Kconfig parsing to find
'if' lines
Hello again!
On 24/08/2021 16:30, Masahiro Yamada wrote:
> On Mon, Aug 23, 2021 at 4:22 AM Ariel Marcovitch
> <arielmarcovitch@...il.com> wrote:
>>
>> When parsing Kconfig files to find symbol definitions and references,
>> lines after a 'help' line are skipped until a new config definition
>> starts.
>>
>> However, it is quite common to define a config and then make some other
>> configs depend on it by adding an 'if' line. This kind of kconfig
>> statement usually appears after a config definition which might contain
>> a 'help' section. The 'if' line is skipped in parse_kconfig_file()
>> because it is not a config definition.
>>
>> This means that symbols referenced in this kind of statements are
>> ignored by this function and thus are not considered undefined
>> references in case the symbol is not defined.
>>
>> The REGEX_KCONFIG_STMT regex can't be used because the other types of
>> statements can't break help lines.
>>
>> Define a new regex for matching 'if' statements and stop the 'help'
>> skipping in case it is encountered.
>>
>> Signed-off-by: Ariel Marcovitch <arielmarcovitch@...il.com>
>> ---
>> scripts/checkkconfigsymbols.py | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/checkkconfigsymbols.py
b/scripts/checkkconfigsymbols.py
>> index b9b0f15e5880..875e9a2c14b2 100755
>> --- a/scripts/checkkconfigsymbols.py
>> +++ b/scripts/checkkconfigsymbols.py
>> @@ -26,6 +26,7 @@ EXPR = r"(?:" + OPERATORS + r"|\s|" + SYMBOL + r")+"
>> DEFAULT = r"default\s+.*?(?:if\s.+){,1}"
>> STMT = r"^\s*(?:if|select|imply|depends\s+on|(?:" + DEFAULT +
r"))\s+" + EXPR
>> SOURCE_SYMBOL = r"(?:\W|\b)+[D]{,1}CONFIG_(" + SYMBOL + r")"
>> +IF_LINE = r"^\s*(?:if)\s+" + EXPR
>
>
> Why is it enclosed by "(?: )" ?
>
> "(?:if)" seems to the same as "if"
Oh you are absolutely right.
I just mindlessly copied the STMT regex and removed the other types :)
>
>
>
>
>
>
>>
>> # regex objects
>> REGEX_FILE_KCONFIG = re.compile(r".*Kconfig[\.\w+\-]*$")
>> @@ -35,11 +36,11 @@ REGEX_KCONFIG_DEF = re.compile(DEF)
>> REGEX_KCONFIG_EXPR = re.compile(EXPR)
>> REGEX_KCONFIG_STMT = re.compile(STMT)
>> REGEX_KCONFIG_HELP = re.compile(r"^\s+help\s*$")
>> +REGEX_KCONFIG_IF_LINE = re.compile(IF_LINE)
>> REGEX_FILTER_SYMBOLS = re.compile(r"[A-Za-z0-9]$")
>> REGEX_NUMERIC = re.compile(r"0[xX][0-9a-fA-F]+|[0-9]+")
>> REGEX_QUOTES = re.compile("(\"(.*?)\")")
>>
>> -
>> def parse_options():
>> """The user interface of this module."""
>> usage = "Run this tool to detect Kconfig symbols that are
referenced but " \
>> @@ -445,6 +446,11 @@ def parse_kconfig_file(kfile):
>> line = line.strip('\n')
>> line = line.split("#")[0] # ignore comments
>>
>> + # 'if EXPR' lines can be after help lines
>> + # The if line itself is handled later
>> + if REGEX_KCONFIG_IF_LINE.match(line):
>> + skip = False
>> +
>
>
> I do not think this is the right fix.
> There are similar patterns where
> config references are ignored.
>
> For example, FOO and BAR are ignored
> in the following cases.
>
> ex1)
>
> choice
> prompt "foo"
> default FOO
>
>
>
> ex2)
>
> menu "bar"
> depends on BAR
>
>
>
>
> The help block ends with shallower indentation.
So IIUC we need to measure the indentation when we encounter a help
statement and in the next lines look for a line with a different depth
(which is not an empty line because these are allowed).
>
>
>
>
>> if REGEX_KCONFIG_DEF.match(line):
>> symbol_def = REGEX_KCONFIG_DEF.findall(line)
>> defined.append(symbol_def[0])
>> --
>> 2.25.1
>>
>
>
> --
> Best Regards
> Masahiro Yamada
Thanks for your time!
Ariel Marcovitch
Powered by blists - more mailing lists