| Message ID | 20230809002436.18079-1-sunying@nj.iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kconfig: add dependency warning print about invalid values in verbose mode | expand |
On Wed, Aug 9, 2023 at 9:32 AM <sunying@nj.iscas.ac.cn> wrote: > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > 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. > > 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@buaa.edu.cn> > Signed-off-by: Ying Sun <sunying@nj.iscas.ac.cn> 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"}; > 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]); > + 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"); > + 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'}; > > /* 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, > + 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, > + 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 >
On Sat, Aug 19, 2023 at 8:59 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Aug 9, 2023 at 9:32 AM <sunying@nj.iscas.ac.cn> wrote: > > > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > > > 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@chromium.org/T/#u https://lore.kernel.org/lkml/CAMuHMdWF0LseSGK6=aodXaoK9D16mxok4DDRs=gKoGox8k6zjg@mail.gmail.com/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@buaa.edu.cn> > > Signed-off-by: Ying Sun <sunying@nj.iscas.ac.cn> > > > 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
On (23/08/19 22:40), Jesse T wrote: > > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > > > > > 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. Are those really errors? ERROR : CLANG_VERSION[140006 => 0] value is invalid due to it has default value ERROR : CC_IS_GCC[n => y] value is invalid due to it has default value ERROR : GCC_VERSION[0 => 120200] value is invalid due to it has default value I'm using clang, so corresponding options are set accordingly in my .config
On Wed, Aug 23, 2023 at 10:56 AM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (23/08/19 22:40), Jesse T wrote: > > > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > > > > > > > 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. > > Are those really errors? > > ERROR : CLANG_VERSION[140006 => 0] value is invalid due to it has default value > ERROR : CC_IS_GCC[n => y] value is invalid due to it has default value > ERROR : GCC_VERSION[0 => 120200] value is invalid due to it has default value > > I'm using clang, so corresponding options are set accordingly in my .config I think not errors, but warnings. His idea is, CONFIG options that change a value should be warned. -- Best Regards Masahiro Yamada
On (23/08/24 06:52), Masahiro Yamada wrote: > On Wed, Aug 23, 2023 at 10:56 AM Sergey Senozhatsky > <senozhatsky@chromium.org> wrote: > > > > On (23/08/19 22:40), Jesse T wrote: > > > > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > > > > > > > > > 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. > > > > Are those really errors? > > > > ERROR : CLANG_VERSION[140006 => 0] value is invalid due to it has default value > > ERROR : CC_IS_GCC[n => y] value is invalid due to it has default value > > ERROR : GCC_VERSION[0 => 120200] value is invalid due to it has default value > > > > I'm using clang, so corresponding options are set accordingly in my .config > > > I think not errors, but warnings. I agree. Masahiro, do we really need "list missing" to be blocked on this? This patch has been posted on Aug 8 and there was not much progress since then. Can we, perhaps, move "list missing" forward? --- scripts/kconfig/confdata.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index fa2ae6f63352..bf6d473755aa 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -360,7 +360,10 @@ int conf_read_simple(const char *name, int def) char *p, *p2; struct symbol *sym; int i, def_flags; + bool missing = false; + const char *sanity_checks; + sanity_checks = getenv("KCONFIG_SANITY_CHECKS"); if (name) { in = zconf_fopen(name); } else { @@ -448,6 +451,13 @@ int conf_read_simple(const char *name, int def) if (def == S_DEF_USER) { sym = sym_find(line + 2 + strlen(CONFIG_)); if (!sym) { + if (sanity_checks) { + conf_warning("unknown symbol: %s", + line + 2 + strlen(CONFIG_)); + missing = true; + continue; + } + conf_set_changed(true); continue; } @@ -482,6 +492,13 @@ int conf_read_simple(const char *name, int def) sym = sym_find(line + strlen(CONFIG_)); if (!sym) { + if (sanity_checks && def != S_DEF_AUTO) { + conf_warning("unknown symbol: %s", + line + strlen(CONFIG_)); + missing = true; + continue; + } + if (def == S_DEF_AUTO) /* * Reading from include/config/auto.conf @@ -530,6 +547,10 @@ int conf_read_simple(const char *name, int def) } free(line); fclose(in); + + if (missing) + exit(1); + return 0; }
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@gmail.com>wrote: > > On Sat, Aug 19, 2023 at 8:59 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Wed, Aug 9, 2023 at 9:32 AM <sunying@nj.iscas.ac.cn> wrote: > > > > > > From: Ying Sun <sunying@nj.iscas.ac.cn> > > > > > > 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@chromium.org/T/#u > https://lore.kernel.org/lkml/CAMuHMdWF0LseSGK6=aodXaoK9D16mxok4DDRs=gKoGox8k6zjg@mail.gmail.com/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@buaa.edu.cn> > > > Signed-off-by: Ying Sun <sunying@nj.iscas.ac.cn> > > > > > > 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
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"}; 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]); + 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"); + 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'}; /* 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, + 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, + 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]) {