I apologize for this, but I cannot reproduce this issue in my environment. I have tried removing CONFIG_SMP or changing it to n in the arch/x86/configs/x86_64_defconfig file,and none of them resulted in segfault. I am using the latest Linux kernel version and compilation toolchain. In order to better locate and solve this issue, could you provide more information？ > On Sun, Aug 20, 2023 at 10:40AM Jesse T <mr.bossman075@xxxxxxxxx>wrote: > > On Sat, Aug 19, 2023 at 8:59 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > > > On Wed, Aug 9, 2023 at 9:32 AM <sunying@xxxxxxxxxxxxxx> wrote: > > > > > > From: Ying Sun <sunying@xxxxxxxxxxxxxx> > > > > > > Add warning about the configuration option's invalid value in verbose mode, > > > including error causes, mismatch dependency, old and new values, > > > to help users correct them. > > It should also warn about invalid options which were being talked > about in other places. > https://lore.kernel.org/all/20230817012007.131868-1-senozhatsky@xxxxxxxxxxxx/T/#u > https://lore.kernel.org/lkml/CAMuHMdWF0LseSGK6=aodXaoK9D16mxok4DDRs=gKoGox8k6zjg@xxxxxxxxxxxxxx/T/#mb5b73532166014ce0a66b0e7e11dbe06528d863c > > I can also cause a segfault by removing options such as removing > `CONFIG_SMP=y` from x86_64_defconfig > > > > > > > Detailed error messages are printed only when the environment variable > > > is set like "KCONFIG_VERBOSE=1". > > > By default, the current behavior is not changed. > > > > > > Signed-off-by: Siyuan Guo <zy21df106@xxxxxxxxxxx> > > > Signed-off-by: Ying Sun <sunying@xxxxxxxxxxxxxx> > > > > > > This patch is much bigger than I had expected. > > > > I did not go through all the lines, but > > I think the behavior should be improved. > > > > > > > > [1] A downgrade from 'y' to 'm' should be warned. > > > > > > You only check (sym->dir_dep.tri == no && sym->def[S_DEF_USER].tri != no) > > > > but (sym->dir_dep.tri == mod && sym->def[S_DEF_USER].tri == yes) > > will cause =y to be downgraded to =m. > > This must be warned. > > > > > > > > > > [2] A new CONFIG option should not be warned. > > > > $ make defconfig > > > > Add the following two lines to a Kconfig file. > > > > config THIS_IS_A_NEW_OPTION > > def_bool y > > > > > > $ make KCONFIG_VERBOSE=1 oldconfig > > > > ERROR : THIS_IS_A_NEW_OPTION[n => y] value is invalid > > due to it has default value > > # > > # configuration written to .config > > # > > > > > > > > > > This is totally harmless. > > I do not understand why it is warned. > > > > > > > > > > > > BTW, your patch subject says > > "add dependency warning", but your actually code prints "ERROR". > > > > Also, check the coding style. > > > > > > > > > > > > > > > > > --- > > > scripts/kconfig/confdata.c | 121 +++++++++++++++++++++++++++++++++++-- > > > scripts/kconfig/lkc.h | 3 + > > > scripts/kconfig/symbol.c | 82 +++++++++++++++++++++++-- > > > 3 files changed, 195 insertions(+), 11 deletions(-) > > > > > > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c > > > index 992575f1e976..fa2ae6f63352 100644 > > > --- a/scripts/kconfig/confdata.c > > > +++ b/scripts/kconfig/confdata.c > > > @@ -154,6 +154,7 @@ static void conf_message(const char *fmt, ...) > > > > > > static const char *conf_filename; > > > static int conf_lineno, conf_warnings; > > > +const char *verbose; > > > > > > static void conf_warning(const char *fmt, ...) > > > { > > > @@ -226,7 +227,7 @@ static const char *conf_get_rustccfg_name(void) > > > static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) > > > { > > > char *p2; > > > - > > > + static const char * const type[] = {"unknown", "bool", "tristate", "int", "hex", "string"}; > > Add a new line after the declaration. > Not sure why checkpatch didn't get this. > > > > switch (sym->type) { > > > case S_TRISTATE: > > > if (p[0] == 'm') { > > > @@ -246,9 +247,14 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) > > > sym->flags |= def_flags; > > > break; > > > } > > > - if (def != S_DEF_AUTO) > > > - conf_warning("symbol value '%s' invalid for %s", > > > + if (def != S_DEF_AUTO) { > > > + if (verbose) > > > + conf_warning("symbol value '%s' invalid for %s\n due to its type is %s", > > > + p, sym->name, type[sym->type]); > > normally indented to the opening parenthesis, same with the rest. > > > > + else > > > + conf_warning("symbol value '%s' invalid for %s", > > > p, sym->name); > > > + } > > > return 1; > > > case S_STRING: > > > /* No escaping for S_DEF_AUTO (include/config/auto.conf) */ > > > @@ -274,9 +280,14 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p) > > > sym->def[def].val = xstrdup(p); > > > sym->flags |= def_flags; > > > } else { > > > - if (def != S_DEF_AUTO) > > > - conf_warning("symbol value '%s' invalid for %s", > > > - p, sym->name); > > > + if (def != S_DEF_AUTO) { > > > + if (verbose) > > > + conf_warning("symbol value '%s' invalid for %s\n due to its type is %s", > > > + p, sym->name, type[sym->type]); > > > + else > > > + conf_warning("symbol value '%s' invalid for %s", > > > + p, sym->name); > > > + } > > > return 1; > > > } > > > break; > > > @@ -528,6 +539,7 @@ int conf_read(const char *name) > > > int conf_unsaved = 0; > > > int i; > > > > > > + verbose = getenv("KCONFIG_VERBOSE"); > > > conf_set_changed(false); > > > > > > if (conf_read_simple(name, S_DEF_USER)) { > > > @@ -559,6 +571,103 @@ int conf_read(const char *name) > > > continue; > > > conf_unsaved++; > > > /* maybe print value in verbose mode... */ > > > + if (verbose) { > > > + if (sym_is_choice_value(sym)) { > > > + struct property *prop = sym_get_choice_prop(sym); > > > + struct symbol *defsym = prop_get_symbol(prop)->curr.val; > > > + > > > + if (defsym && defsym != sym) { > > > + struct gstr gs = str_new(); > > > + > > > + str_printf(&gs, > > > + "\nERROR : %s[%c => %c] value is invalid\n", > > > + sym->name, > > > + tristate2char[sym->def[S_DEF_USER].tri], > > > + tristate2char[sym->curr.tri]); > > > + str_printf(&gs, > > > + " due to its not the choice default symbol\n"); > > > + str_printf(&gs, > > > + " the default symbol is %s\n", > > > + defsym->name); > > > + fputs(str_get(&gs), stderr); > > > + } > > > + } else { > > > + switch (sym->type) { > > > + case S_BOOLEAN: > > > + case S_TRISTATE: > > > + if (sym->dir_dep.tri == no && > > > + sym->def[S_DEF_USER].tri != no) { > > > + struct gstr gs = str_new(); > > > + > > > + str_printf(&gs, > > > + "\nERROR: unmet direct dependencies detected for %s[%c => %c]\n", > > > + sym->name, > > > + tristate2char[sym->def[S_DEF_USER].tri], > > > + tristate2char[sym->curr.tri]); > > > + str_printf(&gs, > > > + " Depends on [%c]: ", > > > + sym->dir_dep.tri == mod ? 'm' : 'n'); > > > + expr_gstr_print(sym->dir_dep.expr, &gs); > > > + str_printf(&gs, "\n"); > > > + fputs(str_get(&gs), stderr); > > > + } else if (sym->rev_dep.tri != no) { > > > + struct gstr gs = str_new(); > > > + > > > + str_printf(&gs, > > > + "\nERROR : %s[%c => %c] value is invalid\n", > > > + sym->name, > > > + tristate2char[sym->def[S_DEF_USER].tri], > > > + tristate2char[sym->curr.tri]); > > > + str_printf(&gs, > > > + " due to its invisible and it is selected\n"); > > As Masahiro said this is unnecessary. > Maybe add a verbosity level for this. > > > > + expr_gstr_print_revdep(sym->rev_dep.expr, &gs, yes, > > > + " Selected by [y]:\n"); > > > + expr_gstr_print_revdep(sym->rev_dep.expr, &gs, mod, > > > + " Selected by [m]:\n"); > > > + fputs(str_get(&gs), stderr); > > > + } else { > > > + sym_validate_default(sym); > > > + } > > > + break; > > > + case S_INT: > > > + case S_HEX: > > > + if (sym->dir_dep.tri == no && > > > + strcmp((char *)(sym->def[S_DEF_USER].val), "") != 0) { > > > + struct gstr gs = str_new(); > > > + > > > + str_printf(&gs, > > > + "\nERROR: unmet direct dependencies detected for %s\n", > > > + sym->name); > > > + str_printf(&gs, > > > + " Depends on [%c]: ", > > > + sym->dir_dep.tri == mod ? 'm' : 'n'); > > > + expr_gstr_print(sym->dir_dep.expr, &gs); > > > + str_printf(&gs, "\n"); > > > + fputs(str_get(&gs), stderr); > > > + } else { > > > + sym_validate_default(sym); > > > + } > > > + break; > > > + case S_STRING: > > > + if (sym->dir_dep.tri == no && > > > + strcmp((char *)(sym->def[S_DEF_USER].val), "") != 0) { > > > + struct gstr gs = str_new(); > > > + > > > + str_printf(&gs, > > > + "\nERROR: unmet direct dependencies detected for %s\n", > > > + sym->name); > > > + str_printf(&gs, > > > + " Depends on [%c]: ", > > > + sym->dir_dep.tri == mod ? 'm' : 'n'); > > > + expr_gstr_print(sym->dir_dep.expr, &gs); > > > + str_printf(&gs, "\n"); > > > + fputs(str_get(&gs), stderr); > > > + } > > > + default: > > > + break; > > > + } > > > + } > > > + } > > > } > > > > > > for_all_symbols(i, sym) { > > > diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h > > > index 471a59acecec..820a47fb4968 100644 > > > --- a/scripts/kconfig/lkc.h > > > +++ b/scripts/kconfig/lkc.h > > > @@ -38,6 +38,8 @@ void zconf_initscan(const char *name); > > > void zconf_nextfile(const char *name); > > > int zconf_lineno(void); > > > const char *zconf_curname(void); > > > +extern const char *verbose; > > > +static const char tristate2char[3] = {'n', 'm', 'y'}; > > This seems non-ideal > > > > > > > /* confdata.c */ > > > const char *conf_get_configname(void); > > > @@ -112,6 +114,7 @@ struct property *sym_get_range_prop(struct symbol *sym); > > > const char *sym_get_string_default(struct symbol *sym); > > > struct symbol *sym_check_deps(struct symbol *sym); > > > struct symbol *prop_get_symbol(struct property *prop); > > > +void sym_validate_default(struct symbol *sym); > > > > > > static inline tristate sym_get_tristate_value(struct symbol *sym) > > > { > > > diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c > > > index 0572330bf8a7..8b11d6ea1d30 100644 > > > --- a/scripts/kconfig/symbol.c > > > +++ b/scripts/kconfig/symbol.c > > > @@ -91,6 +91,53 @@ static struct property *sym_get_default_prop(struct symbol *sym) > > > return NULL; > > > } > > > > > > +void sym_validate_default(struct symbol *sym) > > > +{ > > > + if (sym->visible == no) { > > > + struct gstr gs = str_new(); > > > + const char *value = sym_get_string_default(sym); > > > + > > > + switch (sym->type) { > > > + case S_BOOLEAN: > > > + case S_TRISTATE: > > > + if (strcmp(value, "n") != 0) { > > > + str_printf(&gs, > > > + "\nERROR : %s[%c => %c] value is invalid\n due to it has default value\n", > > > + sym->name, > Same here > > > + tristate2char[sym->def[S_DEF_USER].tri], > > > + tristate2char[sym->curr.tri]); > > > + } else if (sym->implied.tri != no) { > > > + str_printf(&gs, > > > + "\nERROR : %s[%c => %c] value is invalid\n due to its invisible and has imply value\n", > > > + sym->name, > And here > > > + tristate2char[sym->def[S_DEF_USER].tri], > > > + tristate2char[sym->curr.tri]); > > > + str_printf(&gs, > > > + " Imply : "); > > > + expr_gstr_print(sym->implied.expr, &gs); > > > + str_printf(&gs, "\n"); > > > + } > > > + break; > > > + case S_STRING: > > > + case S_INT: > > > + case S_HEX: > > > + if (strcmp(value, "") != 0) { > > > + str_printf(&gs, > > > + "\nERROR : %s[%s => %s] value is invalid\n", > > > + sym->name, > > > + (char *)(sym->def[S_DEF_USER].val), > > > + (char *)(sym->curr.val)); > > > + str_printf(&gs, > > > + " due to it has default value\n"); > > > + } > > > + break; > > > + default: > > > + break; > > > + } > > > + fputs(str_get(&gs), stderr); > > > + } > > > +} > > > + > > > struct property *sym_get_range_prop(struct symbol *sym) > > > { > > > struct property *prop; > > > @@ -600,7 +647,8 @@ bool sym_string_valid(struct symbol *sym, const char *str) > > > bool sym_string_within_range(struct symbol *sym, const char *str) > > > { > > > struct property *prop; > > > - long long val; > > > + long long val, left, right; > > > + struct gstr gs = str_new(); > > > > > > switch (sym->type) { > > > case S_STRING: > > > @@ -612,8 +660,20 @@ bool sym_string_within_range(struct symbol *sym, const char *str) > > > if (!prop) > > > return true; > > > val = strtoll(str, NULL, 10); > > > - return val >= sym_get_range_val(prop->expr->left.sym, 10) && > > > - val <= sym_get_range_val(prop->expr->right.sym, 10); > > > + left = sym_get_range_val(prop->expr->left.sym, 10); > > > + right = sym_get_range_val(prop->expr->right.sym, 10); > > > + if (val >= left && val <= right) > > > + return true; > > > + if (verbose) { > > > + str_printf(&gs, > > > + "\nERROR: unmet range detected for %s\n", > > > + sym->name); > > > + str_printf(&gs, > > > + " symbol value is %lld, the range is (%lld %lld)\n", > > > + val, left, right); > > > + fputs(str_get(&gs), stderr); > > > + } > > > + return false; > > > case S_HEX: > > > if (!sym_string_valid(sym, str)) > > > return false; > > > @@ -621,8 +681,20 @@ bool sym_string_within_range(struct symbol *sym, const char *str) > > > if (!prop) > > > return true; > > > val = strtoll(str, NULL, 16); > > > - return val >= sym_get_range_val(prop->expr->left.sym, 16) && > > > - val <= sym_get_range_val(prop->expr->right.sym, 16); > > > + left = sym_get_range_val(prop->expr->left.sym, 16); > > > + right = sym_get_range_val(prop->expr->right.sym, 16); > > > + if (val >= left && val <= right) > > > + return true; > > > + if (verbose) { > > > + str_printf(&gs, > > > + "\nERROR: unmet range detected for %s\n", > > > + sym->name); > > > + str_printf(&gs, > > > + " symbol value is 0x%llx, the range is (0x%llx 0x%llx)\n", > > > + val, left, right); > > > + fputs(str_get(&gs), stderr); > > > + } > > > + return false; > > > case S_BOOLEAN: > > > case S_TRISTATE: > > > switch (str[0]) { > > > -- > > > 2.17.1 > > > > > > > > > -- > > Best Regards > > Masahiro Yamada
