| Message ID | 20240418101903.11314-1-sunying@isrc.iscas.ac.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv4,next] kconfig: add dependency warning print about invalid values while KBUILD_EXTRA_WARN=c | expand |
On Thu, Apr 18, 2024 at 7:19 PM <sunying@isrc.iscas.ac.cn> wrote: > > From: Ying Sun <sunying@isrc.iscas.ac.cn> > > The patch enhances kernel error messages, fixes problems with > the previous version of v3, Unneeded information. You do not need to advertise your previous wrong patch. The patch submission history should be described below the '---' marker. > and has been thoroughly tested. Unneeded. It is quite normal for a patch submitter to test their patch before submission. > We believe it will improve the clarity and usefulness > of kconfig error messages, which will help developers better diagnose and > resolve configuration issues. "We believe" is unneeded. > > ----- v3 -> v4: > 1. Fixed the dependency logic print error, distinguishing > between unsatisfied dependencies and forced enable > errors (related to the select keyword). > 2. Add export KCONFIG_WARN_CHANGED_INPUT=1 to scripts/kconfig/Makefile, > which can be enabled by setting KBUILD_EXTRA_WARN to -c. > > Signed-off-by: Siyuan Guo <zy21df106@buaa.edu.cn> > Signed-off-by: Ying Sun <sunying@isrc.iscas.ac.cn> > --- > scripts/kconfig/Makefile | 1 + > scripts/kconfig/confdata.c | 60 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile > index ea1bf3b3dbde..b755246fe057 100644 > --- a/scripts/kconfig/Makefile > +++ b/scripts/kconfig/Makefile > @@ -29,6 +29,7 @@ KCONFIG_DEFCONFIG_LIST += arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) > > ifneq ($(findstring c, $(KBUILD_EXTRA_WARN)),) > export KCONFIG_WARN_UNKNOWN_SYMBOLS=1 > +export KCONFIG_WARN_CHANGED_INPUT=1 > endif > > ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),) > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c > index 0e35c4819cf1..91c63bd1cedd 100644 > --- a/scripts/kconfig/confdata.c > +++ b/scripts/kconfig/confdata.c > @@ -176,6 +176,41 @@ static void conf_warning(const char *fmt, ...) > conf_warnings++; > } > > +static void conf_warn_unmet_rev(struct symbol *sym) > +{ > + struct gstr gs = str_new(); > + > + str_printf(&gs, > + "WARNING: unmet reverse dependencies detected for %s\n", > + sym->name); This message is wrong. Kconfig changed the value so it meets the reverse dependency. It should warn that the value has been changed. > + > + 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); > + str_free(&gs); > +} > + > +static void conf_warn_dep_error(struct symbol *sym) > +{ > + struct gstr gs = str_new(); > + > + str_printf(&gs, > + "WARNING: unmet direct dependencies detected for %s\n", > + sym->name); Same here. Kconfig changed the value so it meets the direct dependency. > + > + 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); > + str_free(&gs); > +} > + > static void conf_default_message_callback(const char *s) > { > printf("#\n# "); > @@ -522,6 +557,31 @@ int conf_read(const char *name) > continue; > conf_unsaved++; > /* maybe print value in verbose mode... */ > + if (getenv("KCONFIG_WARN_CHANGED_INPUT")) { > + if (sym->prop) { Why did you check sym->prop here? > + switch (sym->type) { > + case S_BOOLEAN: > + case S_TRISTATE: > + if (sym->def[S_DEF_USER].tri != sym_get_tristate_value(sym)) { > + if (sym->rev_dep.tri < sym->def[S_DEF_USER].tri && > + sym->dir_dep.tri < sym->def[S_DEF_USER].tri) > + conf_warn_dep_error(sym); > + if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri && > + sym->dir_dep.tri >= sym->def[S_DEF_USER].tri) > + conf_warn_unmet_rev(sym); This is clumsy. conf_warn_dep_error() and conf_warn_unmet_rev() do not happen at the same time. if (sym->def[S_DEF_USER].tri != sym_get_tristate_value(sym)) { if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri) conf_warn_unmet_rev(sym); else if (sym->dir_dep.tri < sym->def[S_DEF_USER].tri) conf_warn_dep_error(sym); } is much simpler. > + } > + break; > + case S_INT: > + case S_HEX: > + case S_STRING: > + if (sym->dir_dep.tri == no && sym_has_value(sym)) > + conf_warn_dep_error(sym); > + break; > + default: > + break; > + } > + } > + } > } > > for_all_symbols(sym) { > -- > 2.43.0 > >
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index ea1bf3b3dbde..b755246fe057 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -29,6 +29,7 @@ KCONFIG_DEFCONFIG_LIST += arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) ifneq ($(findstring c, $(KBUILD_EXTRA_WARN)),) export KCONFIG_WARN_UNKNOWN_SYMBOLS=1 +export KCONFIG_WARN_CHANGED_INPUT=1 endif ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),) diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 0e35c4819cf1..91c63bd1cedd 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -176,6 +176,41 @@ static void conf_warning(const char *fmt, ...) conf_warnings++; } +static void conf_warn_unmet_rev(struct symbol *sym) +{ + struct gstr gs = str_new(); + + str_printf(&gs, + "WARNING: unmet reverse dependencies detected for %s\n", + sym->name); + + 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); + str_free(&gs); +} + +static void conf_warn_dep_error(struct symbol *sym) +{ + struct gstr gs = str_new(); + + str_printf(&gs, + "WARNING: 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); + str_free(&gs); +} + static void conf_default_message_callback(const char *s) { printf("#\n# "); @@ -522,6 +557,31 @@ int conf_read(const char *name) continue; conf_unsaved++; /* maybe print value in verbose mode... */ + if (getenv("KCONFIG_WARN_CHANGED_INPUT")) { + if (sym->prop) { + switch (sym->type) { + case S_BOOLEAN: + case S_TRISTATE: + if (sym->def[S_DEF_USER].tri != sym_get_tristate_value(sym)) { + if (sym->rev_dep.tri < sym->def[S_DEF_USER].tri && + sym->dir_dep.tri < sym->def[S_DEF_USER].tri) + conf_warn_dep_error(sym); + if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri && + sym->dir_dep.tri >= sym->def[S_DEF_USER].tri) + conf_warn_unmet_rev(sym); + } + break; + case S_INT: + case S_HEX: + case S_STRING: + if (sym->dir_dep.tri == no && sym_has_value(sym)) + conf_warn_dep_error(sym); + break; + default: + break; + } + } + } } for_all_symbols(sym) {