Skip to content

Commit edda15f

Browse files
tchebbmasahir0y
authored andcommitted
kconfig: list all definitions of a symbol in help text
In Kconfig, each symbol (representing a config option) can be defined in multiple places. Each definition may or may not have a prompt, which allows the option to be set via an interface like menuconfig. Each definition has a set of dependencies, which determine whether its prompt is visible and whether other pieces of the definition, like a default value, take effect. Historically, a symbol's help text (i.e. what's shown when a user presses '?' in menuconfig) contained some symbol-wide information not tied to any particular definition (e.g. what other symbols it selects) as well as the location (file name and line number) and dependencies of each prompt. Notably, the help text did not show the location or dependencies of definitions without prompts. Because this made it hard to reason about symbols that had no prompts, commit bcdedcc ("menuconfig: print more info for symbol without prompts") changed the help text so that, instead of containing the location and dependencies of each prompt, it contained the location and dependencies of the symbol's first definition, regardless of whether or not that definition had a prompt. For symbols with only one definition, that change makes sense. However, it breaks down for symbols with multiple definitions: each definition has its own set of dependencies (the `dep` field of `struct menu`), and those dependencies are ORed together to get the symbol's dependency list (the `dir_dep` field of `struct symbol`). By printing only the dependencies of the first definition, the help text misleads users into believing that an option is more narrowly-applicable than it actually is. For an extreme example of this, we can look at the SYS_TEXT_BASE symbol in the Das U-Boot project (version 2019.10), which also uses Kconfig. (I unfortunately could not find an illustrative example in Linux.) This config option specifies the load address of the built binary and, as such, is applicable to basically every configuration possible. And yet, without this patch, its help text is as follows: Symbol: SYS_TEXT_BASE [=] Type : hex Prompt: U-Boot base address Location: -> ARM architecture Prompt: Text Base Location: -> Boot images Defined at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=n] && ARCH_ASPEED [=n] The help text indicates that the option is applicable only for a specific unselected architecture (aspeed), because that architecture's promptless definition (which just sets a default value), happens to be the first one seen. No definition or dependency information is printed for either of the two prompts listed. Because source locations and dependencies are fundamentally properties of definitions and not of symbols, we should treat them as such. This patch brings back the pre-bcdedcc1afd6 behavior for definitions with prompts but also separately prints the location and dependencies of those without prompts, solving the original problem in a different way. With this change, our SYS_TEXT_BASE example becomes Symbol: SYS_TEXT_BASE [=] Type : hex Defined at arch/arm/mach-stm32mp/Kconfig:83 Prompt: U-Boot base address Depends on: ARM [=n] && ARCH_STM32MP [=n] Location: -> ARM architecture Defined at Kconfig:532 Prompt: Text Base Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] Location: -> Boot images Defined at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=n] && ARCH_ASPEED [=n] Defined at arch/arm/mach-socfpga/Kconfig:25 Depends on: ARM [=n] && ARCH_SOCFPGA [=n] <snip> Defined at board/sifive/fu540/Kconfig:15 Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] which is a much more accurate representation. Note that there is one notable difference between what gets printed for prompts after this change and what got printed before bcdedcc: the "Depends on" line now accurately represents the prompt's dependencies instead of conflating those with the prompt's visibility (which can include extra conditions). See the patch later in this series titled "kconfig: distinguish between dependencies and visibility in help text" for more details and better handling of that nuance. Signed-off-by: Thomas Hebb <[email protected]> Signed-off-by: Masahiro Yamada <[email protected]>
1 parent 89b9060 commit edda15f

File tree

1 file changed

+31
-24
lines changed

1 file changed

+31
-24
lines changed

scripts/kconfig/menu.c

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -711,14 +711,31 @@ const char *menu_get_help(struct menu *menu)
711711
return "";
712712
}
713713

714+
static void get_def_str(struct gstr *r, struct menu *menu)
715+
{
716+
str_printf(r, "Defined at %s:%d\n",
717+
menu->file->name, menu->lineno);
718+
}
719+
720+
static void get_dep_str(struct gstr *r, struct expr *expr, const char *prefix)
721+
{
722+
if (!expr_is_yes(expr)) {
723+
str_append(r, prefix);
724+
expr_gstr_print(expr, r);
725+
str_append(r, "\n");
726+
}
727+
}
728+
714729
static void get_prompt_str(struct gstr *r, struct property *prop,
715730
struct list_head *head)
716731
{
717732
int i, j;
718733
struct menu *submenu[8], *menu, *location = NULL;
719734
struct jump_key *jump = NULL;
720735

721-
str_printf(r, "Prompt: %s\n", prop->text);
736+
str_printf(r, " Prompt: %s\n", prop->text);
737+
738+
get_dep_str(r, prop->menu->dep, " Depends on: ");
722739
menu = prop->menu->parent;
723740
for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) {
724741
bool accessible = menu_is_visible(menu);
@@ -768,18 +785,6 @@ static void get_prompt_str(struct gstr *r, struct property *prop,
768785
}
769786
}
770787

771-
/*
772-
* get property of type P_SYMBOL
773-
*/
774-
static struct property *get_symbol_prop(struct symbol *sym)
775-
{
776-
struct property *prop = NULL;
777-
778-
for_all_properties(sym, prop, P_SYMBOL)
779-
break;
780-
return prop;
781-
}
782-
783788
static void get_symbol_props_str(struct gstr *r, struct symbol *sym,
784789
enum prop_type tok, const char *prefix)
785790
{
@@ -819,17 +824,19 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,
819824
}
820825
}
821826
}
822-
for_all_prompts(sym, prop)
823-
get_prompt_str(r, prop, head);
824-
825-
prop = get_symbol_prop(sym);
826-
if (prop) {
827-
str_printf(r, " Defined at %s:%d\n", prop->menu->file->name,
828-
prop->menu->lineno);
829-
if (!expr_is_yes(prop->visible.expr)) {
830-
str_append(r, " Depends on: ");
831-
expr_gstr_print(prop->visible.expr, r);
832-
str_append(r, "\n");
827+
828+
/* Print the definitions with prompts before the ones without */
829+
for_all_properties(sym, prop, P_SYMBOL) {
830+
if (prop->menu->prompt) {
831+
get_def_str(r, prop->menu);
832+
get_prompt_str(r, prop->menu->prompt, head);
833+
}
834+
}
835+
836+
for_all_properties(sym, prop, P_SYMBOL) {
837+
if (!prop->menu->prompt) {
838+
get_def_str(r, prop->menu);
839+
get_dep_str(r, prop->menu->dep, " Depends on: ");
833840
}
834841
}
835842

0 commit comments

Comments
 (0)