2020-08-17 09:55:15

by Jerome Forissier

[permalink] [raw]
Subject: [PATCH] checkpatch: get CONFIG_ prefix from the environment

Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
To make this possible, update checkpatch to use the value of $CONFIG_
if defined or "CONFIG_" otherwise.

Signed-off-by: Jerome Forissier <[email protected]>
---
scripts/checkpatch.pl | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..2cf750175a71 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my $CONFIG_ = $ENV{"CONFIG_"} || "CONFIG_";

sub help {
my ($exitcode) = @_;
@@ -6528,16 +6529,16 @@ sub process {
}

# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^$CONFIG_/) {
WARN("IS_ENABLED_CONFIG",
- "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+ "IS_ENABLED($1) is normally used as IS_ENABLED($CONFIG_$1)\n" . $herecurr);
}

# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
- if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
+ if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
- "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
+ "Prefer IS_ENABLED(<FOO>) to $CONFIG_<FOO> || $CONFIG_<FOO>_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
}
--
2.25.1


2020-08-17 14:10:51

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: get CONFIG_ prefix from the environment

On Mon, 2020-08-17 at 11:50 +0200, Jerome Forissier wrote:
> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
> environment variable. Out-of-tree projects may therefore use Kconfig
> with a different prefix, or they may use a custom configuration tool
> which does not use the CONFIG_ prefix at all. Such projects may still
> want to adhere to the Linux kernel coding style and run checkpatch.pl.
> To make this possible, update checkpatch to use the value of $CONFIG_
> if defined or "CONFIG_" otherwise.
>
> Signed-off-by: Jerome Forissier <[email protected]>
> ---
> scripts/checkpatch.pl | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2cbeae6d9aee..2cf750175a71 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
> # git output parsing needs US English output, so first set backtick child process LANGUAGE
> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
> my $tabsize = 8;
> +my $CONFIG_ = $ENV{"CONFIG_"} || "CONFIG_";

I'm not a big fan of environment variable being
used to control program behavior.

Maybe add something to .checkpatch.conf instead.

> sub help {
> my ($exitcode) = @_;
> @@ -6528,16 +6529,16 @@ sub process {
> }
>
> # check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
> - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
> + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^$CONFIG_/) {
> WARN("IS_ENABLED_CONFIG",
> - "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
> + "IS_ENABLED($1) is normally used as IS_ENABLED($CONFIG_$1)\n" . $herecurr);
> }
>
> # check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
> - if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> my $config = $1;
> if (WARN("PREFER_IS_ENABLED",
> - "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
> + "Prefer IS_ENABLED(<FOO>) to $CONFIG_<FOO> || $CONFIG_<FOO>_MODULE\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
> }

2020-08-17 14:29:29

by Jerome Forissier

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: get CONFIG_ prefix from the environment



On 8/17/20 4:09 PM, Joe Perches wrote:
> On Mon, 2020-08-17 at 11:50 +0200, Jerome Forissier wrote:
>> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
>> environment variable. Out-of-tree projects may therefore use Kconfig
>> with a different prefix, or they may use a custom configuration tool
>> which does not use the CONFIG_ prefix at all. Such projects may still
>> want to adhere to the Linux kernel coding style and run checkpatch.pl.
>> To make this possible, update checkpatch to use the value of $CONFIG_
>> if defined or "CONFIG_" otherwise.
>>
>> Signed-off-by: Jerome Forissier <[email protected]>
>> ---
>> scripts/checkpatch.pl | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 2cbeae6d9aee..2cf750175a71 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
>> # git output parsing needs US English output, so first set backtick child process LANGUAGE
>> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
>> my $tabsize = 8;
>> +my $CONFIG_ = $ENV{"CONFIG_"} || "CONFIG_";
>
> I'm not a big fan of environment variable being
> used to control program behavior.

OK.

>
> Maybe add something to .checkpatch.conf instead.

That would work equally well for me. I will post a V2.

Thanks,
--
Jerome

>
>> sub help {
>> my ($exitcode) = @_;
>> @@ -6528,16 +6529,16 @@ sub process {
>> }
>>
>> # check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
>> - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
>> + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^$CONFIG_/) {
>> WARN("IS_ENABLED_CONFIG",
>> - "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
>> + "IS_ENABLED($1) is normally used as IS_ENABLED($CONFIG_$1)\n" . $herecurr);
>> }
>>
>> # check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
>> - if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
>> + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
>> my $config = $1;
>> if (WARN("PREFER_IS_ENABLED",
>> - "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
>> + "Prefer IS_ENABLED(<FOO>) to $CONFIG_<FOO> || $CONFIG_<FOO>_MODULE\n" . $herecurr) &&
>> $fix) {
>> $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
>> }
>

2020-08-17 19:28:56

by Jerome Forissier

[permalink] [raw]
Subject: [PATCH v2] checkpatch: add --kconfig-prefix

Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.
To make this possible, add the --kconfig-prefix command line option.

Signed-off-by: Jerome Forissier <[email protected]>
---
scripts/checkpatch.pl | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..150dfbc04b4b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my $CONFIG_ = "CONFIG_";

sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ $CONFIG_)
-h, --help, --version display this help and exit

When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \$CONFIG_,
'h|help' => \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}

# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^$CONFIG_/) {
WARN("IS_ENABLED_CONFIG",
- "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+ "IS_ENABLED($1) is normally used as IS_ENABLED($CONFIG_$1)\n" . $herecurr);
}

# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
- if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
+ if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)($CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
- "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
+ "Prefer IS_ENABLED(<FOO>) to $CONFIG_<FOO> || $CONFIG_<FOO>_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
}
--
2.25.1

2020-08-18 07:45:49

by Jerome Forissier

[permalink] [raw]
Subject: [PATCH v3] checkpatch: add --kconfig-prefix

Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.

One example is OP-TEE [1] which does not use Kconfig but does have
configuration options prefixed with CFG_. It also mostly follows the
kernel coding style and therefore being able to use checkpatch is quite
valuable.

To make this possible, add the --kconfig-prefix command line option.

Signed-off-by: Jerome Forissier <[email protected]>
---
scripts/checkpatch.pl | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

v3:
- Use ${CONFIG_} instead of $CONFIG_.
- Expand the commit message to mention OP-TEE.

v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..fd65f8c774ed 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my ${CONFIG_} = "CONFIG_";

sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ ${CONFIG_})
-h, --help, --version display this help and exit

When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \${CONFIG_},
'h|help' => \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}

# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^${CONFIG_}/) {
WARN("IS_ENABLED_CONFIG",
- "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+ "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
}

# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
- if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
+ if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
- "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
+ "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
}
--
2.25.1

2020-08-18 07:51:46

by Jerome Forissier

[permalink] [raw]
Subject: Re: [PATCH v3] checkpatch: add --kconfig-prefix



On 8/18/20 9:43 AM, Jerome Forissier wrote:
> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
> environment variable. Out-of-tree projects may therefore use Kconfig
> with a different prefix, or they may use a custom configuration tool
> which does not use the CONFIG_ prefix at all. Such projects may still
> want to adhere to the Linux kernel coding style and run checkpatch.pl.
>
> One example is OP-TEE [1] which does not use Kconfig but does have
> configuration options prefixed with CFG_. It also mostly follows the
> kernel coding style and therefore being able to use checkpatch is quite
> valuable.
>
> To make this possible, add the --kconfig-prefix command line option.

(Oh I forgot to add the link here :-/ sorry about that. Let me know if
you want me to resend.)

[1] https://github.com/OP-TEE/optee_os

>
> Signed-off-by: Jerome Forissier <[email protected]>
> ---
> scripts/checkpatch.pl | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> v3:
> - Use ${CONFIG_} instead of $CONFIG_.
> - Expand the commit message to mention OP-TEE.
>
> v2:
> - Use a command-line/.checkpatch.conf option instead of the _CONFIG
> environment variable.
> - Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
> the environment").
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2cbeae6d9aee..fd65f8c774ed 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
> # git output parsing needs US English output, so first set backtick child process LANGUAGE
> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
> my $tabsize = 8;
> +my ${CONFIG_} = "CONFIG_";
>
> sub help {
> my ($exitcode) = @_;
> @@ -127,6 +128,8 @@ Options:
> --typedefsfile Read additional types from this file
> --color[=WHEN] Use colors 'always', 'never', or only when output
> is a terminal ('auto'). Default is 'auto'.
> + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
> + ${CONFIG_})
> -h, --help, --version display this help and exit
>
> When FILE is - read standard input.
> @@ -235,6 +238,7 @@ GetOptions(
> 'color=s' => \$color,
> 'no-color' => \$color, #keep old behaviors of -nocolor
> 'nocolor' => \$color, #keep old behaviors of -nocolor
> + 'kconfig-prefix=s' => \${CONFIG_},
> 'h|help' => \$help,
> 'version' => \$help
> ) or help(1);
> @@ -6528,16 +6532,16 @@ sub process {
> }
>
> # check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
> - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
> + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^${CONFIG_}/) {
> WARN("IS_ENABLED_CONFIG",
> - "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
> + "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
> }
>
> # check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
> - if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> my $config = $1;
> if (WARN("PREFER_IS_ENABLED",
> - "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
> + "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
> }
>

2020-08-18 08:04:42

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v3] checkpatch: add --kconfig-prefix

On Tue, 2020-08-18 at 09:43 +0200, Jerome Forissier wrote:
> Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
> environment variable. Out-of-tree projects may therefore use Kconfig
> with a different prefix, or they may use a custom configuration tool
> which does not use the CONFIG_ prefix at all. Such projects may still
> want to adhere to the Linux kernel coding style and run checkpatch.pl.
>
> One example is OP-TEE [1] which does not use Kconfig but does have
> configuration options prefixed with CFG_. It also mostly follows the
> kernel coding style and therefore being able to use checkpatch is quite
> valuable.
>
> To make this possible, add the --kconfig-prefix command line option.
>
> Signed-off-by: Jerome Forissier <[email protected]>

Acked-by: Joe Perches <[email protected]>

> ---
> scripts/checkpatch.pl | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> v3:
> - Use ${CONFIG_} instead of $CONFIG_.
> - Expand the commit message to mention OP-TEE.

Thanks for the nit-work...

cheers, Joe

> v2:
> - Use a command-line/.checkpatch.conf option instead of the _CONFIG
> environment variable.
> - Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
> the environment").
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2cbeae6d9aee..fd65f8c774ed 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
> # git output parsing needs US English output, so first set backtick child process LANGUAGE
> my $git_command ='export LANGUAGE=en_US.UTF-8; git';
> my $tabsize = 8;
> +my ${CONFIG_} = "CONFIG_";
>
> sub help {
> my ($exitcode) = @_;
> @@ -127,6 +128,8 @@ Options:
> --typedefsfile Read additional types from this file
> --color[=WHEN] Use colors 'always', 'never', or only when output
> is a terminal ('auto'). Default is 'auto'.
> + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
> + ${CONFIG_})
> -h, --help, --version display this help and exit
>
> When FILE is - read standard input.
> @@ -235,6 +238,7 @@ GetOptions(
> 'color=s' => \$color,
> 'no-color' => \$color, #keep old behaviors of -nocolor
> 'nocolor' => \$color, #keep old behaviors of -nocolor
> + 'kconfig-prefix=s' => \${CONFIG_},
> 'h|help' => \$help,
> 'version' => \$help
> ) or help(1);
> @@ -6528,16 +6532,16 @@ sub process {
> }
>
> # check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
> - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
> + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^${CONFIG_}/) {
> WARN("IS_ENABLED_CONFIG",
> - "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
> + "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
> }
>
> # check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
> - if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
> my $config = $1;
> if (WARN("PREFER_IS_ENABLED",
> - "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
> + "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
> }

2020-08-18 08:08:55

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v3] checkpatch: add --kconfig-prefix

On Tue, 2020-08-18 at 09:50 +0200, Jerome Forissier wrote:
>
> On 8/18/20 9:43 AM, Jerome Forissier wrote:
> > Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
> > environment variable. Out-of-tree projects may therefore use Kconfig
> > with a different prefix, or they may use a custom configuration tool
> > which does not use the CONFIG_ prefix at all. Such projects may still
> > want to adhere to the Linux kernel coding style and run checkpatch.pl.
> >
> > One example is OP-TEE [1] which does not use Kconfig but does have
> > configuration options prefixed with CFG_. It also mostly follows the
> > kernel coding style and therefore being able to use checkpatch is quite
> > valuable.
> >
> > To make this possible, add the --kconfig-prefix command line option.
>
> (Oh I forgot to add the link here :-/ sorry about that. Let me know if
> you want me to resend.)
>
> [1] https://github.com/OP-TEE/optee_os

It's probably not important enough to bother.
A trivial search on "OP-TEE" works.
So I think it's fine, but if you feel like it,
go ahead.

Joe


2020-08-18 08:21:08

by Jerome Forissier

[permalink] [raw]
Subject: [PATCH v4] checkpatch: add --kconfig-prefix

Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable. Out-of-tree projects may therefore use Kconfig
with a different prefix, or they may use a custom configuration tool
which does not use the CONFIG_ prefix at all. Such projects may still
want to adhere to the Linux kernel coding style and run checkpatch.pl.

One example is OP-TEE [1] which does not use Kconfig but does have
configuration options prefixed with CFG_. It also mostly follows the
kernel coding style and therefore being able to use checkpatch is quite
valuable.

To make this possible, add the --kconfig-prefix command line option.

[1] https://github.com/OP-TEE/optee_os

Signed-off-by: Jerome Forissier <[email protected]>
Acked-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

v4:
- Add missing link
- Apply Joe's Acked-by: tag

v3:
- Use ${CONFIG_} instead of $CONFIG_.
- Expand the commit message to mention OP-TEE.

v2:
- Use a command-line/.checkpatch.conf option instead of the _CONFIG
environment variable.
- Changed the patch subject (was: "checkpatch: get CONFIG_ prefix from
the environment").

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2cbeae6d9aee..fd65f8c774ed 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC
# git output parsing needs US English output, so first set backtick child process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
my $tabsize = 8;
+my ${CONFIG_} = "CONFIG_";

sub help {
my ($exitcode) = @_;
@@ -127,6 +128,8 @@ Options:
--typedefsfile Read additional types from this file
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
+ --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default
+ ${CONFIG_})
-h, --help, --version display this help and exit

When FILE is - read standard input.
@@ -235,6 +238,7 @@ GetOptions(
'color=s' => \$color,
'no-color' => \$color, #keep old behaviors of -nocolor
'nocolor' => \$color, #keep old behaviors of -nocolor
+ 'kconfig-prefix=s' => \${CONFIG_},
'h|help' => \$help,
'version' => \$help
) or help(1);
@@ -6528,16 +6532,16 @@ sub process {
}

# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too)
- if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) {
+ if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^${CONFIG_}/) {
WARN("IS_ENABLED_CONFIG",
- "IS_ENABLED($1) is normally used as IS_ENABLED(CONFIG_$1)\n" . $herecurr);
+ "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr);
}

# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE
- if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
+ if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(${CONFIG_}[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) {
my $config = $1;
if (WARN("PREFER_IS_ENABLED",
- "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) &&
+ "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] = "\+#if IS_ENABLED($config)";
}
--
2.25.1