[PATCH] kconfig: Warn if help text is blank

[PATCH] kconfig: Warn if help text is blank

[Ulf Magnusson]

Print a warning if a 'help' token is given but the help text is blank.
Personal pet peeve.

Example warnings:

	net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text
	net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text
	net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text
	drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text
	drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text
	drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text
	drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text
	lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text

A separate patchset will be sent to fix all current instances of blank
help texts for all arches. I added the same warning to Kconfiglib.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/zconf.y | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
index 21ce883e5d9e..4be98050b961 100644
--- a/scripts/kconfig/zconf.y
+++ b/scripts/kconfig/zconf.y
@@ -436,6 +436,12 @@ help: help_start T_HELPTEXT
 		zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used",
 			   current_entry->sym->name ?: "<choice>");
 	}
+
+	/* Is the help text empty or all whitespace? */
+	if ($2[strspn($2, " \f\n\r\t\v")] == '\0')
+		zconfprint("warning: '%s' defined with blank help text",
+			   current_entry->sym->name ?: "<choice>");
+
 	current_entry->help = $2;
 };
 
-- 
2.14.1

Re: [PATCH] kconfig: Warn if help text is blank

[Paul Bolle]

On Tue, 2018-01-30 at 19:18 +0100, Ulf Magnusson wrote:
> Print a warning if a 'help' token is given but the help text is blank.
> Personal pet peeve.
> 
> Example warnings:
> 
> 	net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text
> 	net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text
> 	net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text
> 	drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text
> 	drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text
> 	drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text
> 	drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text
> 	lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text
> 
> A separate patchset will be sent to fix all current instances of blank
> help texts for all arches. I added the same warning to Kconfiglib.

If you do this it would be better to first fix or remove those help texts, and
only then add this warning. Ie, add the warning in the last patch of a cleanup
series.

> --- a/scripts/kconfig/zconf.y
> +++ b/scripts/kconfig/zconf.y
> @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT
>  		zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used",
>  			   current_entry->sym->name ?: "<choice>");
>  	}
> +
> +	/* Is the help text empty or all whitespace? */
> +	if ($2[strspn($2, " \f\n\r\t\v")] == '\0')
> +		zconfprint("warning: '%s' defined with blank help text",
> +			   current_entry->sym->name ?: "<choice>");
> +

Does this go to stderr?

Another fix would be to ignore empty help texts and not render them at all. Is
that possible?

Thanks,


Paul Bolle

Re: [PATCH] kconfig: Warn if help text is blank

[Ulf Magnusson]

On Tue, Jan 30, 2018 at 7:34 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Tue, 2018-01-30 at 19:18 +0100, Ulf Magnusson wrote:
>> Print a warning if a 'help' token is given but the help text is blank.
>> Personal pet peeve.
>>
>> Example warnings:
>>
>>       net/sched/Kconfig:860: warning: 'NET_IFE_SKBMARK' defined with blank help text
>>       net/sched/Kconfig:865: warning: 'NET_IFE_SKBPRIO' defined with blank help text
>>       net/sched/Kconfig:870: warning: 'NET_IFE_SKBTCINDEX' defined with blank help text
>>       drivers/video/fbdev/Kconfig:1159: warning: 'FB_I810_I2C' defined with blank help text
>>       drivers/mmc/host/Kconfig:877: warning: 'MMC_TOSHIBA_PCI' defined with blank help text
>>       drivers/staging/rtl8192u/Kconfig:8: warning: 'RTL8192U' defined with blank help text
>>       drivers/staging/rtl8192e/rtl8192e/Kconfig:9: warning: 'RTL8192E' defined with blank help text
>>       lib/Kconfig.debug:354: warning: 'ARCH_WANT_FRAME_POINTERS' defined with blank help text
>>
>> A separate patchset will be sent to fix all current instances of blank
>> help texts for all arches. I added the same warning to Kconfiglib.
>
> If you do this it would be better to first fix or remove those help texts

I agree that this shouldn't go in until/unless a significant portion
of those empty help texts get removed first.

> and only then add this warning. Ie, add the warning in the last patch of a cleanup
> series.

Might have been better to arrange it like that, yeah. The patchset
that removes the empty help texts is at
https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd
prefer that.

>
>> --- a/scripts/kconfig/zconf.y
>> +++ b/scripts/kconfig/zconf.y
>> @@ -436,6 +436,12 @@ help: help_start T_HELPTEXT
>>               zconfprint("warning: '%s' defined with more than one help text -- only the last one will be used",
>>                          current_entry->sym->name ?: "<choice>");
>>       }
>> +
>> +     /* Is the help text empty or all whitespace? */
>> +     if ($2[strspn($2, " \f\n\r\t\v")] == '\0')
>> +             zconfprint("warning: '%s' defined with blank help text",
>> +                        current_entry->sym->name ?: "<choice>");
>> +
>
> Does this go to stderr?

Yep - goes to stderr. zconfprint() is the standard error reporting
function during parsing, and also prints the parsing location.

>
> Another fix would be to ignore empty help texts and not render them at all. Is
> that possible?

I haven't even checked how they are rendered to be honest. I was more
concerned with the Kconfig mess.

> Thanks,
>
>
> Paul Bolle

Cheers,
Ulf

Re: [PATCH] kconfig: Warn if help text is blank

[Paul Bolle]

On Tue, 2018-01-30 at 20:33 +0100, Ulf Magnusson wrote:
> I agree that this shouldn't go in until/unless a significant portion
> of those empty help texts get removed first.

Not a significant portion, but all, I'd say.

> The patchset that removes the empty help texts is at
> https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd
> prefer that.

I'm fine with anything that doesn't add warnings just to appease pet peeves.

> I haven't even checked how they are rendered to be honest. I was more
> concerned with the Kconfig mess.

Well, if you could make empty help texts disappear somehow (either during the
Kconfig parse phase - which requires a jump into the yacc horror - or during
the rendering phase) that would be much better than adding a warning for
something that you yourself described as a pet peeve.

And after that's done you're free to send cleanup patches for this whenever
you feel like doing so.

Thanks,


Paul Bolle

Re: [PATCH] kconfig: Warn if help text is blank

[Ulf Magnusson]

On Tue, Jan 30, 2018 at 9:31 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Tue, 2018-01-30 at 20:33 +0100, Ulf Magnusson wrote:
>> I agree that this shouldn't go in until/unless a significant portion
>> of those empty help texts get removed first.
>
> Not a significant portion, but all, I'd say.
>
>> The patchset that removes the empty help texts is at
>> https://lkml.org/lkml/2018/1/30/574. I could make another one if you'd
>> prefer that.
>
> I'm fine with anything that doesn't add warnings just to appease pet peeves.
>
>> I haven't even checked how they are rendered to be honest. I was more
>> concerned with the Kconfig mess.
>
> Well, if you could make empty help texts disappear somehow (either during the
> Kconfig parse phase - which requires a jump into the yacc horror - or during
> the rendering phase) that would be much better than adding a warning for
> something that you yourself described as a pet peeve.
>
> And after that's done you're free to send cleanup patches for this whenever
> you feel like doing so.
>
> Thanks,
>
>
> Paul Bolle

I rolled a new patchset which adds the warning at the end and makes
the commits that remove the blank help texts more freestanding:
https://lkml.org/lkml/2018/1/31/137

Cheers,
Ulf