* [PATCHv4 next] kconfig: add dependency warning print about invalid values while KBUILD_EXTRA_WARN=c
@ 2024-04-18 10:19 sunying
2024-04-26 18:49 ` Masahiro Yamada
0 siblings, 1 reply; 2+ messages in thread
From: sunying @ 2024-04-18 10:19 UTC (permalink / raw)
To: masahiroy; +Cc: linux-kbuild, linux-kernel, pengpeng, zy21df106, Ying Sun
From: Ying Sun <sunying@isrc.iscas.ac.cn>
The patch enhances kernel error messages, fixes problems with
the previous version of v3, and has been thoroughly tested.
We believe it will improve the clarity and usefulness
of kconfig error messages, which will help developers better diagnose and
resolve configuration issues.
----- 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);
+
+ 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) {
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCHv4 next] kconfig: add dependency warning print about invalid values while KBUILD_EXTRA_WARN=c
2024-04-18 10:19 [PATCHv4 next] kconfig: add dependency warning print about invalid values while KBUILD_EXTRA_WARN=c sunying
@ 2024-04-26 18:49 ` Masahiro Yamada
0 siblings, 0 replies; 2+ messages in thread
From: Masahiro Yamada @ 2024-04-26 18:49 UTC (permalink / raw)
To: sunying; +Cc: linux-kbuild, linux-kernel, pengpeng, zy21df106
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
>
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-26 18:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-18 10:19 [PATCHv4 next] kconfig: add dependency warning print about invalid values while KBUILD_EXTRA_WARN=c sunying
2024-04-26 18:49 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).