Skip to contentSkip to conversation

kconfig: Warn if help text is blank

PATCH In review Ulf Magnusson 2 participants 5 messages Jan 30, 2018 – Jan 31, 2018

Subsystems (2)
Kbuild Kconfig
lore
Share & tools
lore

Ulf Magnusson

author
#0 lore raw

PATCH 1/1 kconfig: Warn if help text is blank

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 <[email protected]>
---
 scripts/kconfig/zconf.y | 6 ++++++
 1 file changed, 6 insertions(+)
scripts/kconfig/zconf.y +6 -0
index 21ce883e5d9e..4be98050b961 100644
@@ -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;
 };
Signature · 1 line
2.14.1
#1 lore raw
Quoted 14 lines · 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

Quoted 2 lines

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.

scripts/kconfig/zconf.y help: help_start T_HELPTEXT · line 444

Quoted 12 lines
scripts/kconfig/zconf.y +6 -0
@@ -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

author
#2 lore raw
Quoted 15 lines · Paul Bolle
Quoted 14 lines · 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

Quoted 4 lines
Quoted 2 lines

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.

Quoted 2 lines

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.

Quoted 15 lines

scripts/kconfig/zconf.y help: help_start T_HELPTEXT · line 444

Quoted 13 lines
scripts/kconfig/zconf.y +6 -0
@@ -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.

Quoted 3 lines

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.

Quoted 4 lines

Thanks,

Paul Bolle

Cheers,
Ulf

#3 lore raw
Quoted 2 lines · Ulf Magnusson

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.

Quoted 3 lines

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.

Quoted 2 lines

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

author
#4 lore raw
Quoted 20 lines · Paul Bolle
Quoted 2 lines · Ulf Magnusson

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.

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 7 lines

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