All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
@ 2018-02-16 20:22 Ulf Magnusson
  2018-02-16 20:22 ` [PATCH 1/3] checkpatch: kconfig: recognize more prompts when checking help texts Ulf Magnusson
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ulf Magnusson @ 2018-02-16 20:22 UTC (permalink / raw)
  To: apw, joe; +Cc: linux-kernel, linux-kbuild, Ulf Magnusson

Hello,

This patchset contains some improvements for the Kconfig help text check in
scripts/checkconfig.pl:

 - Commits 1 and 2 make the check more robust by checking menuconfig symbols
   and choices as well as symbols defined with 'config', and by making the
   detection of definition boundaries more reliable.

 - Commit 3 adds a check for new '---help---'s being introduced. IMO, 'help'
   should be encouraged in new code.

All three commits can be applied independently.

The existing code is a bit weird in that it doesn't require symbols with "long"
definitions (e.g., many selects) to have help texts. Fixing that is outside the
scope of this patchset. I couldn't tell if it was deliberate.

I'm a Perl noob, so check for bad practices. :)

The changes were tested by running 'checkpatch.pl -f' on some large existing
Kconfig files in the kernel and looking for false positives (e.g.
arch/{x86,arm}/Kconfig).

This test file was also used, which contains some cases that confused the old
code:

	config BAD_1
		bool "bad 1"
	
	config BAD_2
		bool 'bad 2'
	
	config BAD_3
		bool "bad 3"
		help
		  1
		  2
		  3
	
	menuconfig BAD_4
		bool "bad 4"
		help
		  1
		  2
		  3
	
	config BAD_5
		bool
		prompt "bad 5"
		help
		  1
		  2
		  3
	
	config BAD_6
		bool "bad 6"
		help
		  1
		  2
		  3
	
	if FOO
	
	config BAD_7
		bool "bad 7"
		help
		  1
		  2
		  3
	
	endif
	
	config BAD_8
		bool "bad 8"
		help
		  1
		  2
		  3
	
	source "foo"
	
	config BAD_9
		bool "bad 9"
		---help---
		  1
		  2
		  3
		  4
	
	choice
		bool "bad choice"
		help
		  1
		  2
		  3
	
	endchoice
	
	config OK_1
		bool
	
	config OK_2
		bool "ok 2"
		help
		  1
		  2
		  3
		  4
	
	config OK_3
		tristate "ok 3"
		help
		  1
		  2
		  3
		  4
	
	config OK_4
		tristate
		prompt "ok 4"
		help
		  1
		  2
		  3
		  4
	
	choice
		bool "ok choice"
		help
		  1
		  2
		  3
		  4
	
	endchoice


This now produces the following warnings:

	WARNING: please write a paragraph that describes the config symbol fully
	#9: FILE: Kconfig.test_help_check:9:
	+config BAD_1
	
	WARNING: please write a paragraph that describes the config symbol fully
	#12: FILE: Kconfig.test_help_check:12:
	+config BAD_2
	
	WARNING: please write a paragraph that describes the config symbol fully
	#15: FILE: Kconfig.test_help_check:15:
	+config BAD_3
	
	WARNING: please write a paragraph that describes the config symbol fully
	#22: FILE: Kconfig.test_help_check:22:
	+menuconfig BAD_4
	
	WARNING: please write a paragraph that describes the config symbol fully
	#29: FILE: Kconfig.test_help_check:29:
	+config BAD_5
	
	WARNING: please write a paragraph that describes the config symbol fully
	#37: FILE: Kconfig.test_help_check:37:
	+config BAD_6
	
	WARNING: please write a paragraph that describes the config symbol fully
	#46: FILE: Kconfig.test_help_check:46:
	+config BAD_7
	
	WARNING: please write a paragraph that describes the config symbol fully
	#55: FILE: Kconfig.test_help_check:55:
	+config BAD_8
	
	WARNING: prefer 'help' over '---help---' for new help texts
	#64: FILE: Kconfig.test_help_check:64:
	+config BAD_9
	
	WARNING: please write a paragraph that describes the config symbol fully
	#72: FILE: Kconfig.test_help_check:72:
	+choice
	
	total: 0 errors, 10 warnings, 117 lines checked
	
	NOTE: For some of the reported defects, checkpatch may be able to
	      mechanically convert to the typical style using --fix or --fix-inplace.
	
	Kconfig.test_help_check has style problems, please review.
	
	NOTE: If any of the errors are false positives, please report
	      them to the maintainer, see CHECKPATCH in MAINTAINERS.


Cheers,
Ulf

Ulf Magnusson (3):
  checkpatch: kconfig: recognize more prompts when checking help texts
  checkpatch: kconfig: check help texts for menuconfig and choice
  checkpatch: kconfig: prefer 'help' over '---help---'

 scripts/checkpatch.pl | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] checkpatch: kconfig: recognize more prompts when checking help texts
  2018-02-16 20:22 [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Ulf Magnusson
@ 2018-02-16 20:22 ` Ulf Magnusson
  2018-02-16 20:22 ` [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice Ulf Magnusson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Ulf Magnusson @ 2018-02-16 20:22 UTC (permalink / raw)
  To: apw, joe; +Cc: linux-kernel, linux-kbuild, Ulf Magnusson

The check for a missing or short help text only considers symbols with a
prompt, but doesn't recognize any of the following as a prompt:

	bool 'foo'
	tristate 'foo'
	prompt "foo"
	prompt 'foo'

Make the check recognize those too.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d4040322ae1..2b404317daea 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2812,7 +2812,7 @@ sub process {
 				next if ($f =~ /^-/);
 				last if (!$file && $f =~ /^\@\@/);
 
-				if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate)\s*\"/) {
+				if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
 					$is_start = 1;
 				} elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) {
 					$length = -1;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice
  2018-02-16 20:22 [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Ulf Magnusson
  2018-02-16 20:22 ` [PATCH 1/3] checkpatch: kconfig: recognize more prompts when checking help texts Ulf Magnusson
@ 2018-02-16 20:22 ` Ulf Magnusson
  2018-03-22 15:13   ` Masahiro Yamada
  2018-02-16 20:22 ` [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---' Ulf Magnusson
  2018-02-16 21:14 ` [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Joe Perches
  3 siblings, 1 reply; 12+ messages in thread
From: Ulf Magnusson @ 2018-02-16 20:22 UTC (permalink / raw)
  To: apw, joe; +Cc: linux-kernel, linux-kbuild, Ulf Magnusson

Currently, only Kconfig symbols are checked for a missing or short help
text, and are only checked if they are defined with the 'config'
keyword.

To make the check more general, extend it to also check help texts for
choices and for symbols defined with the 'menuconfig' keyword.

This increases the accuracy of the check for symbols that would already
have been checked as well, since e.g. a 'menuconfig' symbol after a help
text will be recognized as ending the preceding symbol/choice
definition.

To increase the accuracy of the check further, also recognize 'if',
'endif', 'menu', 'endmenu', 'endchoice', and 'source' as ending a
symbol/choice definition.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/checkpatch.pl | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2b404317daea..54b782fab4fd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2797,7 +2797,10 @@ sub process {
 # Only applies when adding the entry originally, after that we do not have
 # sufficient context to determine whether it is indeed long enough.
 		if ($realfile =~ /Kconfig/ &&
-		    $line =~ /^\+\s*config\s+/) {
+		    # 'choice' is usually the last thing on the line (though
+		    # Kconfig supports named choices), so use a word boundary
+		    # (\b) rather than a whitespace character (\s)
+		    $line =~ /^\+\s*(?:config|menuconfig|choice)\b/) {
 			my $length = 0;
 			my $cnt = $realcnt;
 			my $ln = $linenr + 1;
@@ -2822,7 +2825,13 @@ sub process {
 				$f =~ s/#.*//;
 				$f =~ s/^\s+//;
 				next if ($f =~ /^$/);
-				if ($f =~ /^\s*config\s/) {
+
+				# This only checks context lines in the patch
+				# and so hopefully shouldn't trigger false
+				# positives, even though some of these are
+				# common words in help texts
+				if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice|
+						  if|endif|menu|endmenu|source)\b/x) {
 					$is_end = 1;
 					last;
 				}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---'
  2018-02-16 20:22 [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Ulf Magnusson
  2018-02-16 20:22 ` [PATCH 1/3] checkpatch: kconfig: recognize more prompts when checking help texts Ulf Magnusson
  2018-02-16 20:22 ` [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice Ulf Magnusson
@ 2018-02-16 20:22 ` Ulf Magnusson
  2018-03-22 15:19   ` Masahiro Yamada
  2018-02-16 21:14 ` [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Joe Perches
  3 siblings, 1 reply; 12+ messages in thread
From: Ulf Magnusson @ 2018-02-16 20:22 UTC (permalink / raw)
  To: apw, joe; +Cc: linux-kernel, linux-kbuild, Ulf Magnusson

IMO, we should discourage '---help---' for new help texts, even in cases
where it would be consistent with other help texts in the file. This
will help if we ever want to get rid of '---help---' in the future.

Also simplify the code to only check for exactly '---help---'. Since
commit c2264564df3d ("kconfig: warn of unhandled characters in Kconfig
commands"), '---help---' is a proper keyword and can only appear in that
form. Prior to that commit, '---help---' working was more of a syntactic
quirk.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/checkpatch.pl | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 54b782fab4fd..2784f6ab309f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2817,7 +2817,11 @@ sub process {
 
 				if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
 					$is_start = 1;
-				} elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) {
+				} elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) {
+					if ($lines[$ln - 1] =~ "---help---") {
+						WARN("CONFIG_DESCRIPTION",
+						     "prefer 'help' over '---help---' for new help texts\n" . $herecurr);
+					}
 					$length = -1;
 				}
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-02-16 20:22 [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Ulf Magnusson
                   ` (2 preceding siblings ...)
  2018-02-16 20:22 ` [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---' Ulf Magnusson
@ 2018-02-16 21:14 ` Joe Perches
  2018-02-23  1:30   ` Ulf Magnusson
  3 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2018-02-16 21:14 UTC (permalink / raw)
  To: Ulf Magnusson, apw, Andrew Morton; +Cc: linux-kernel, linux-kbuild

On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
> Hello,
> 
> This patchset contains some improvements for the Kconfig help text check in
> scripts/checkconfig.pl:

Seems sensible enough to me.
Signed-off-by: Joe Perches <joe@perches.com>

>  - Commits 1 and 2 make the check more robust by checking menuconfig symbols
>    and choices as well as symbols defined with 'config', and by making the
>    detection of definition boundaries more reliable.
> 
>  - Commit 3 adds a check for new '---help---'s being introduced. IMO, 'help'
>    should be encouraged in new code.
> 
> All three commits can be applied independently.
> 
> The existing code is a bit weird in that it doesn't require symbols with "long"
> definitions (e.g., many selects) to have help texts. Fixing that is outside the
> scope of this patchset. I couldn't tell if it was deliberate.
> 
> I'm a Perl noob, so check for bad practices. :)

Everyone is.  Seems fine.

> The changes were tested by running 'checkpatch.pl -f' on some large existing
> Kconfig files in the kernel and looking for false positives (e.g.
> arch/{x86,arm}/Kconfig).
> 
> This test file was also used, which contains some cases that confused the old
> code:
> 
> 	config BAD_1
> 		bool "bad 1"
> 	
> 	config BAD_2
> 		bool 'bad 2'
> 	
> 	config BAD_3
> 		bool "bad 3"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	menuconfig BAD_4
> 		bool "bad 4"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	config BAD_5
> 		bool
> 		prompt "bad 5"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	config BAD_6
> 		bool "bad 6"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	if FOO
> 	
> 	config BAD_7
> 		bool "bad 7"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	endif
> 	
> 	config BAD_8
> 		bool "bad 8"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	source "foo"
> 	
> 	config BAD_9
> 		bool "bad 9"
> 		---help---
> 		  1
> 		  2
> 		  3
> 		  4
> 	
> 	choice
> 		bool "bad choice"
> 		help
> 		  1
> 		  2
> 		  3
> 	
> 	endchoice
> 	
> 	config OK_1
> 		bool
> 	
> 	config OK_2
> 		bool "ok 2"
> 		help
> 		  1
> 		  2
> 		  3
> 		  4
> 	
> 	config OK_3
> 		tristate "ok 3"
> 		help
> 		  1
> 		  2
> 		  3
> 		  4
> 	
> 	config OK_4
> 		tristate
> 		prompt "ok 4"
> 		help
> 		  1
> 		  2
> 		  3
> 		  4
> 	
> 	choice
> 		bool "ok choice"
> 		help
> 		  1
> 		  2
> 		  3
> 		  4
> 	
> 	endchoice
> 
> 
> This now produces the following warnings:
> 
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#9: FILE: Kconfig.test_help_check:9:
> 	+config BAD_1
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#12: FILE: Kconfig.test_help_check:12:
> 	+config BAD_2
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#15: FILE: Kconfig.test_help_check:15:
> 	+config BAD_3
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#22: FILE: Kconfig.test_help_check:22:
> 	+menuconfig BAD_4
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#29: FILE: Kconfig.test_help_check:29:
> 	+config BAD_5
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#37: FILE: Kconfig.test_help_check:37:
> 	+config BAD_6
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#46: FILE: Kconfig.test_help_check:46:
> 	+config BAD_7
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#55: FILE: Kconfig.test_help_check:55:
> 	+config BAD_8
> 	
> 	WARNING: prefer 'help' over '---help---' for new help texts
> 	#64: FILE: Kconfig.test_help_check:64:
> 	+config BAD_9
> 	
> 	WARNING: please write a paragraph that describes the config symbol fully
> 	#72: FILE: Kconfig.test_help_check:72:
> 	+choice
> 	
> 	total: 0 errors, 10 warnings, 117 lines checked
> 	
> 	NOTE: For some of the reported defects, checkpatch may be able to
> 	      mechanically convert to the typical style using --fix or --fix-inplace.
> 	
> 	Kconfig.test_help_check has style problems, please review.
> 	
> 	NOTE: If any of the errors are false positives, please report
> 	      them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> 
> Cheers,
> Ulf
> 
> Ulf Magnusson (3):
>   checkpatch: kconfig: recognize more prompts when checking help texts
>   checkpatch: kconfig: check help texts for menuconfig and choice
>   checkpatch: kconfig: prefer 'help' over '---help---'
> 
>  scripts/checkpatch.pl | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-02-16 21:14 ` [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Joe Perches
@ 2018-02-23  1:30   ` Ulf Magnusson
  2018-02-24 13:53     ` Masahiro Yamada
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Magnusson @ 2018-02-23  1:30 UTC (permalink / raw)
  To: Joe Perches
  Cc: Robo Bot, Andrew Morton, Linux Kernel Mailing List,
	Linux Kbuild mailing list, Masahiro Yamada

On Fri, Feb 16, 2018 at 10:14 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
>> Hello,
>>
>> This patchset contains some improvements for the Kconfig help text check in
>> scripts/checkconfig.pl:
>
> Seems sensible enough to me.
> Signed-off-by: Joe Perches <joe@perches.com>

Will you be taking this in yourself?

(Adding Masahiro on CC -- forgot when I sent the patchset.)

Cheers,
Ulf

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-02-23  1:30   ` Ulf Magnusson
@ 2018-02-24 13:53     ` Masahiro Yamada
  2018-03-06  4:52       ` Ulf Magnusson
  0 siblings, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2018-02-24 13:53 UTC (permalink / raw)
  To: Ulf Magnusson
  Cc: Joe Perches, Robo Bot, Andrew Morton, Linux Kernel Mailing List,
	Linux Kbuild mailing list

2018-02-23 10:30 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Fri, Feb 16, 2018 at 10:14 PM, Joe Perches <joe@perches.com> wrote:
>> On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
>>> Hello,
>>>
>>> This patchset contains some improvements for the Kconfig help text check in
>>> scripts/checkconfig.pl:
>>
>> Seems sensible enough to me.
>> Signed-off-by: Joe Perches <joe@perches.com>
>
> Will you be taking this in yourself?
>
> (Adding Masahiro on CC -- forgot when I sent the patchset.)
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


I am not a perl expert, but I have no objection for this series.


Thanks!




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-02-24 13:53     ` Masahiro Yamada
@ 2018-03-06  4:52       ` Ulf Magnusson
  2018-03-06  5:13         ` Masahiro Yamada
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Magnusson @ 2018-03-06  4:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Joe Perches, Robo Bot, Andrew Morton, Linux Kernel Mailing List,
	Linux Kbuild mailing list

On Sat, Feb 24, 2018 at 2:53 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2018-02-23 10:30 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> On Fri, Feb 16, 2018 at 10:14 PM, Joe Perches <joe@perches.com> wrote:
>>> On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
>>>> Hello,
>>>>
>>>> This patchset contains some improvements for the Kconfig help text check in
>>>> scripts/checkconfig.pl:
>>>
>>> Seems sensible enough to me.
>>> Signed-off-by: Joe Perches <joe@perches.com>
>>
>> Will you be taking this in yourself?
>>
>> (Adding Masahiro on CC -- forgot when I sent the patchset.)
>>
>> Cheers,
>> Ulf
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> I am not a perl expert, but I have no objection for this series.
>
>
> Thanks!
>
>
>
>
> --
> Best Regards
> Masahiro Yamada

*Bump*

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-03-06  4:52       ` Ulf Magnusson
@ 2018-03-06  5:13         ` Masahiro Yamada
  2018-03-22 15:09           ` Masahiro Yamada
  0 siblings, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2018-03-06  5:13 UTC (permalink / raw)
  To: Ulf Magnusson, Andrew Morton
  Cc: Joe Perches, Robo Bot, Linux Kernel Mailing List,
	Linux Kbuild mailing list

(+To: Andrew)

2018-03-06 13:52 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Sat, Feb 24, 2018 at 2:53 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2018-02-23 10:30 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>> On Fri, Feb 16, 2018 at 10:14 PM, Joe Perches <joe@perches.com> wrote:
>>>> On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
>>>>> Hello,
>>>>>
>>>>> This patchset contains some improvements for the Kconfig help text check in
>>>>> scripts/checkconfig.pl:
>>>>
>>>> Seems sensible enough to me.
>>>> Signed-off-by: Joe Perches <joe@perches.com>
>>>
>>> Will you be taking this in yourself?
>>>
>>> (Adding Masahiro on CC -- forgot when I sent the patchset.)
>>>
>>> Cheers,
>>> Ulf
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>> I am not a perl expert, but I have no objection for this series.
>>
>>
>> Thanks!
>>
>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
> *Bump*


Who is addressed by "*Bump*" ?



I think patches for checkpatch.pl
are supposed to be taken care of by Andrew.

He forwards patches to Linus.


$ git log --no-merges --pretty=fuller scripts/checkpatch.pl  | grep
'Commit:' | sort | uniq -c | sort -nr
    555 Commit:     Linus Torvalds <torvalds@linux-foundation.org>
     16 Commit:     Linus Torvalds <torvalds@woody.linux-foundation.org>
      4 Commit:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
      4 Commit:     Michael S. Tsirkin <mst@redhat.com>
      2 Commit:     Thomas Gleixner <tglx@linutronix.de>
      2 Commit:     Ingo Molnar <mingo@kernel.org>
      2 Commit:     Greg Kroah-Hartman <gregkh@suse.de>
      1 Commit:     Tobin C. Harding <me@tobin.cc>
      1 Commit:     Rob Herring <robh@kernel.org>
      1 Commit:     Petr Mladek <pmladek@suse.com>
      1 Commit:     Michal Marek <mmarek@suse.cz>
      1 Commit:     Mauro Carvalho Chehab <mchehab@s-opensource.com>
      1 Commit:     Masahiro Yamada <yamada.masahiro@socionext.com>
      1 Commit:     Lucas De Marchi <lucas.demarchi@profusion.mobi>
      1 Commit:     Jiri Kosina <jkosina@suse.cz>
      1 Commit:     Dan Williams <dan.j.williams@intel.com>
      1 Commit:     Bjorn Helgaas <bhelgaas@google.com>




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks
  2018-03-06  5:13         ` Masahiro Yamada
@ 2018-03-22 15:09           ` Masahiro Yamada
  0 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-03-22 15:09 UTC (permalink / raw)
  To: Ulf Magnusson, Andrew Morton
  Cc: Joe Perches, Robo Bot, Linux Kernel Mailing List,
	Linux Kbuild mailing list

2018-03-06 14:13 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> (+To: Andrew)
>
> 2018-03-06 13:52 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> On Sat, Feb 24, 2018 at 2:53 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> 2018-02-23 10:30 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>>> On Fri, Feb 16, 2018 at 10:14 PM, Joe Perches <joe@perches.com> wrote:
>>>>> On Fri, 2018-02-16 at 21:22 +0100, Ulf Magnusson wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This patchset contains some improvements for the Kconfig help text check in
>>>>>> scripts/checkconfig.pl:
>>>>>
>>>>> Seems sensible enough to me.
>>>>> Signed-off-by: Joe Perches <joe@perches.com>
>>>>
>>>> Will you be taking this in yourself?
>>>>
>>>> (Adding Masahiro on CC -- forgot when I sent the patchset.)
>>>>
>>>> Cheers,
>>>> Ulf
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>> I am not a perl expert, but I have no objection for this series.
>>>
>>>
>>> Thanks!
>>>
>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Masahiro Yamada
>>
>> *Bump*
>
>
> Who is addressed by "*Bump*" ?
>
>
>
> I think patches for checkpatch.pl
> are supposed to be taken care of by Andrew.
>
> He forwards patches to Linus.
>
>
> $ git log --no-merges --pretty=fuller scripts/checkpatch.pl  | grep
> 'Commit:' | sort | uniq -c | sort -nr
>     555 Commit:     Linus Torvalds <torvalds@linux-foundation.org>
>      16 Commit:     Linus Torvalds <torvalds@woody.linux-foundation.org>
>       4 Commit:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>       4 Commit:     Michael S. Tsirkin <mst@redhat.com>
>       2 Commit:     Thomas Gleixner <tglx@linutronix.de>
>       2 Commit:     Ingo Molnar <mingo@kernel.org>
>       2 Commit:     Greg Kroah-Hartman <gregkh@suse.de>
>       1 Commit:     Tobin C. Harding <me@tobin.cc>
>       1 Commit:     Rob Herring <robh@kernel.org>
>       1 Commit:     Petr Mladek <pmladek@suse.com>
>       1 Commit:     Michal Marek <mmarek@suse.cz>
>       1 Commit:     Mauro Carvalho Chehab <mchehab@s-opensource.com>
>       1 Commit:     Masahiro Yamada <yamada.masahiro@socionext.com>
>       1 Commit:     Lucas De Marchi <lucas.demarchi@profusion.mobi>
>       1 Commit:     Jiri Kosina <jkosina@suse.cz>
>       1 Commit:     Dan Williams <dan.j.williams@intel.com>
>       1 Commit:     Bjorn Helgaas <bhelgaas@google.com>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada


I thought Andrew would pick it up, but
nobody is taking care of this after all.

I queued it up now.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice
  2018-02-16 20:22 ` [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice Ulf Magnusson
@ 2018-03-22 15:13   ` Masahiro Yamada
  0 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-03-22 15:13 UTC (permalink / raw)
  To: Ulf Magnusson
  Cc: Robo Bot, Joe Perches, Linux Kernel Mailing List,
	Linux Kbuild mailing list

2018-02-17 5:22 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> Currently, only Kconfig symbols are checked for a missing or short help
> text, and are only checked if they are defined with the 'config'
> keyword.
>
> To make the check more general, extend it to also check help texts for
> choices and for symbols defined with the 'menuconfig' keyword.
>
> This increases the accuracy of the check for symbols that would already
> have been checked as well, since e.g. a 'menuconfig' symbol after a help
> text will be recognized as ending the preceding symbol/choice
> definition.
>
> To increase the accuracy of the check further, also recognize 'if',
> 'endif', 'menu', 'endmenu', 'endchoice', and 'source' as ending a
> symbol/choice definition.


Currently, this is not checked for the last symbol in a file.
Perhaps, EOF could be also an ending of symbol definition.

This patch is a good improvement enough,
so I queued it up.

If you have an idea for further improvement,
v2 is welcome, of course.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---'
  2018-02-16 20:22 ` [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---' Ulf Magnusson
@ 2018-03-22 15:19   ` Masahiro Yamada
  0 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-03-22 15:19 UTC (permalink / raw)
  To: Ulf Magnusson
  Cc: Robo Bot, Joe Perches, Linux Kernel Mailing List,
	Linux Kbuild mailing list

2018-02-17 5:22 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> IMO, we should discourage '---help---' for new help texts, even in cases
> where it would be consistent with other help texts in the file. This
> will help if we ever want to get rid of '---help---' in the future.
>
> Also simplify the code to only check for exactly '---help---'. Since
> commit c2264564df3d ("kconfig: warn of unhandled characters in Kconfig
> commands"), '---help---' is a proper keyword and can only appear in that
> form. Prior to that commit, '---help---' working was more of a syntactic
> quirk.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>  scripts/checkpatch.pl | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 54b782fab4fd..2784f6ab309f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2817,7 +2817,11 @@ sub process {
>
>                                 if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
>                                         $is_start = 1;
> -                               } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) {
> +                               } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) {
> +                                       if ($lines[$ln - 1] =~ "---help---") {
> +                                               WARN("CONFIG_DESCRIPTION",
> +                                                    "prefer 'help' over '---help---' for new help texts\n" . $herecurr);
> +                                       }
>                                         $length = -1;
>                                 }
>


I applied this, but perhaps the line number for the offending part
could be improved.

The warning looks like this.


WARNING: prefer 'help' over '---help---' for new help texts
#1109: FILE: init/Kconfig:1109:
+config SGETMASK_SYSCALL


The #1109 points to the line of the symbol start.
IMHO, it is even better to point to the '---help---' line.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-03-22 15:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16 20:22 [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Ulf Magnusson
2018-02-16 20:22 ` [PATCH 1/3] checkpatch: kconfig: recognize more prompts when checking help texts Ulf Magnusson
2018-02-16 20:22 ` [PATCH 2/3] checkpatch: kconfig: check help texts for menuconfig and choice Ulf Magnusson
2018-03-22 15:13   ` Masahiro Yamada
2018-02-16 20:22 ` [PATCH 3/3] checkpatch: kconfig: prefer 'help' over '---help---' Ulf Magnusson
2018-03-22 15:19   ` Masahiro Yamada
2018-02-16 21:14 ` [PATCH 0/3] Improve and extend checkpatch.pl Kconfig help text checks Joe Perches
2018-02-23  1:30   ` Ulf Magnusson
2018-02-24 13:53     ` Masahiro Yamada
2018-03-06  4:52       ` Ulf Magnusson
2018-03-06  5:13         ` Masahiro Yamada
2018-03-22 15:09           ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.