* [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
@ 2023-08-09 0:24 sunying
2023-08-19 23:01 ` Masahiro Yamada
2023-09-05 9:43 ` [PATCHv2 -next] " sunying
0 siblings, 2 replies; 10+ messages in thread
From: sunying @ 2023-08-09 0:24 UTC (permalink / raw)
To: masahiroy; +Cc: linux-kbuild, linux-kernel, Ying Sun, Siyuan Guo
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>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-09 0:24 [PATCH] kconfig: add dependency warning print about invalid values in verbose mode sunying
@ 2023-08-19 23:01 ` Masahiro Yamada
2023-08-20 2:40 ` Jesse T
2023-09-05 9:43 ` [PATCHv2 -next] " sunying
1 sibling, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2023-08-19 23:01 UTC (permalink / raw)
To: sunying; +Cc: linux-kbuild, linux-kernel, Siyuan Guo
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
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-19 23:01 ` Masahiro Yamada
@ 2023-08-20 2:40 ` Jesse T
2023-08-23 1:55 ` Sergey Senozhatsky
2023-08-29 7:49 ` 郭思远
0 siblings, 2 replies; 10+ messages in thread
From: Jesse T @ 2023-08-20 2:40 UTC (permalink / raw)
To: Masahiro Yamada
Cc: sunying, linux-kbuild, linux-kernel, Siyuan Guo,
Sergey Senozhatsky, Randy Dunlap
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-20 2:40 ` Jesse T
@ 2023-08-23 1:55 ` Sergey Senozhatsky
2023-08-23 21:52 ` Masahiro Yamada
2023-08-29 7:49 ` 郭思远
1 sibling, 1 reply; 10+ messages in thread
From: Sergey Senozhatsky @ 2023-08-23 1:55 UTC (permalink / raw)
To: Ying Sun
Cc: Masahiro Yamada, sunying, linux-kbuild, linux-kernel, Siyuan Guo,
Sergey Senozhatsky, Randy Dunlap, Jesse T
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-23 1:55 ` Sergey Senozhatsky
@ 2023-08-23 21:52 ` Masahiro Yamada
2023-08-24 0:47 ` Sergey Senozhatsky
0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2023-08-23 21:52 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Ying Sun, linux-kbuild, linux-kernel, Siyuan Guo, Randy Dunlap, Jesse T
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-23 21:52 ` Masahiro Yamada
@ 2023-08-24 0:47 ` Sergey Senozhatsky
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2023-08-24 0:47 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Sergey Senozhatsky, Ying Sun, linux-kbuild, linux-kernel,
Siyuan Guo, Randy Dunlap, Jesse T
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;
}
--
2.42.0.rc1.204.g551eb34607-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Re: [PATCH] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-20 2:40 ` Jesse T
2023-08-23 1:55 ` Sergey Senozhatsky
@ 2023-08-29 7:49 ` 郭思远
1 sibling, 0 replies; 10+ messages in thread
From: 郭思远 @ 2023-08-29 7:49 UTC (permalink / raw)
To: Jesse T
Cc: Masahiro Yamada, sunying, linux-kbuild, linux-kernel,
Sergey Senozhatsky, Randy Dunlap, pengpeng
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHv2 -next] kconfig: add dependency warning print about invalid values in verbose mode
2023-08-09 0:24 [PATCH] kconfig: add dependency warning print about invalid values in verbose mode sunying
2023-08-19 23:01 ` Masahiro Yamada
@ 2023-09-05 9:43 ` sunying
2023-11-02 2:50 ` 郭思远
1 sibling, 1 reply; 10+ messages in thread
From: sunying @ 2023-09-05 9:43 UTC (permalink / raw)
To: masahiroy
Cc: linux-kbuild, linux-kernel, senozhatsky, mr.bossman075, Ying Sun,
Siyuan Guo
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>
---
v1 -> v2:
* Reduced the number of code lines by refactoring and simplifying the logic.
* Changed the print "ERROR" to "WARNING".
* Focused on handling dependency errors: dir_dep and rev_dep, and range error.
- A downgrade from 'y' to 'm' has be warned.
- A new CONFIG option should not be warned.
- Overwriting caused by default value is not an error and is no longer printed.
* Fixed style issues.
---
scripts/kconfig/confdata.c | 100 +++++++++++++++++++++++++++++++++++--
scripts/kconfig/lkc.h | 7 +++
scripts/kconfig/symbol.c | 24 +++++++--
3 files changed, 121 insertions(+), 10 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4a6811d77d18..86794ab39d7d 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -154,6 +154,56 @@ static void conf_message(const char *fmt, ...)
static const char *conf_filename;
static int conf_lineno, conf_warnings;
+const char *verbose;
+
+void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...)
+{
+ static char *const tristate2str[3] = {"n", "m", "y"};
+ struct gstr gs = str_new();
+ char s[100];
+ char *oldval = NULL;
+ va_list args;
+
+ va_start(args, log);
+ vsnprintf(s, sizeof(s), log, args);
+ va_end(args);
+
+ switch (sym->type) {
+ case S_BOOLEAN:
+ case S_TRISTATE:
+ oldval = tristate2str[sym->def[S_DEF_USER].tri];
+ break;
+ case S_INT:
+ case S_HEX:
+ case S_STRING:
+ oldval = sym->def[S_DEF_USER].val;
+ default:
+ break;
+ }
+
+ str_printf(&gs,
+ "\nWARNING : %s [%s] value is invalid\n",
+ sym->name, oldval);
+ str_printf(&gs, s);
+ switch (type) {
+ case DIR_DEP:
+ 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");
+ break;
+ case REV_DEP:
+ 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");
+ break;
+ default:
+ break;
+ }
+ fputs(str_get(&gs), stderr);
+}
static void conf_warning(const char *fmt, ...)
{
@@ -226,11 +276,14 @@ 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') {
sym->def[def].tri = mod;
+
+
sym->flags |= def_flags;
break;
}
@@ -246,9 +299,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 +332,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;
@@ -545,6 +608,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)) {
@@ -576,6 +640,32 @@ int conf_read(const char *name)
continue;
conf_unsaved++;
/* maybe print value in verbose mode... */
+ if (verbose) {
+ switch (sym->type) {
+ case S_BOOLEAN:
+ case S_TRISTATE:
+ if (sym->def[S_DEF_USER].tri != sym->curr.tri) {
+ if (sym->dir_dep.tri < sym->def[S_DEF_USER].tri)
+ conf_error_log(DIR_DEP, sym,
+ " due to unmet direct dependencies\n",
+ NULL);
+ if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri)
+ conf_error_log(REV_DEP, sym,
+ " due to it is selected\n", NULL);
+ }
+ break;
+ case S_INT:
+ case S_HEX:
+ case S_STRING:
+ if (sym->dir_dep.tri == no &&
+ strcmp((char *)(sym->def[S_DEF_USER].val), "") != 0)
+ conf_error_log(DIR_DEP, sym,
+ " due to unmet direct dependencies\n", NULL);
+ break;
+ default:
+ break;
+ }
+ }
}
for_all_symbols(i, sym) {
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index 471a59acecec..242b24650f47 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -38,10 +38,17 @@ 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;
+enum error_type {
+ DIR_DEP,
+ REV_DEP,
+ RANGE
+};
/* confdata.c */
const char *conf_get_configname(void);
void set_all_choice_values(struct symbol *csym);
+void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...);
/* confdata.c and expr.c */
static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 0572330bf8a7..a78f7eb64f40 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -600,7 +600,7 @@ 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;
switch (sym->type) {
case S_STRING:
@@ -612,8 +612,15 @@ 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)
+ conf_error_log(RANGE, sym,
+ " symbol value is %lld, the range is (%lld %lld)\n",
+ val, left, right);
+ return false;
case S_HEX:
if (!sym_string_valid(sym, str))
return false;
@@ -621,8 +628,15 @@ 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)
+ conf_error_log(RANGE, sym,
+ " symbol value is 0x%llx, the range is (0x%llx 0x%llx)\n",
+ val, left, right);
+ return false;
case S_BOOLEAN:
case S_TRISTATE:
switch (str[0]) {
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCHv2 -next] kconfig: add dependency warning print about invalid values in verbose mode
2023-09-05 9:43 ` [PATCHv2 -next] " sunying
@ 2023-11-02 2:50 ` 郭思远
2023-11-06 3:51 ` Masahiro Yamada
0 siblings, 1 reply; 10+ messages in thread
From: 郭思远 @ 2023-11-02 2:50 UTC (permalink / raw)
To: masahiroy, linux-kbuild, linux-kernel
Cc: senozhatsky, mr.bossman075, pengpeng, sunying
I am writing to inquire about the status of my patch submission.
Are there any issues with this patch that we need to modify?
> On Fri, Sep 5, 2023 at 5:45PM Ying Sun <sunying@nj.iscas.ac.cn>wrote:
>
> 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>
> ---
> v1 -> v2:
> * Reduced the number of code lines by refactoring and simplifying the logic.
> * Changed the print "ERROR" to "WARNING".
> * Focused on handling dependency errors: dir_dep and rev_dep, and range error.
> - A downgrade from 'y' to 'm' has be warned.
> - A new CONFIG option should not be warned.
> - Overwriting caused by default value is not an error and is no longer printed.
> * Fixed style issues.
> ---
> scripts/kconfig/confdata.c | 100 +++++++++++++++++++++++++++++++++++--
> scripts/kconfig/lkc.h | 7 +++
> scripts/kconfig/symbol.c | 24 +++++++--
> 3 files changed, 121 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 4a6811d77d18..86794ab39d7d 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -154,6 +154,56 @@ static void conf_message(const char *fmt, ...)
>
> static const char *conf_filename;
> static int conf_lineno, conf_warnings;
> +const char *verbose;
> +
> +void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...)
> +{
> + static char *const tristate2str[3] = {"n", "m", "y"};
> + struct gstr gs = str_new();
> + char s[100];
> + char *oldval = NULL;
> + va_list args;
> +
> + va_start(args, log);
> + vsnprintf(s, sizeof(s), log, args);
> + va_end(args);
> +
> + switch (sym->type) {
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + oldval = tristate2str[sym->def[S_DEF_USER].tri];
> + break;
> + case S_INT:
> + case S_HEX:
> + case S_STRING:
> + oldval = sym->def[S_DEF_USER].val;
> + default:
> + break;
> + }
> +
> + str_printf(&gs,
> + "\nWARNING : %s [%s] value is invalid\n",
> + sym->name, oldval);
> + str_printf(&gs, s);
> + switch (type) {
> + case DIR_DEP:
> + 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");
> + break;
> + case REV_DEP:
> + 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");
> + break;
> + default:
> + break;
> + }
> + fputs(str_get(&gs), stderr);
> +}
>
> static void conf_warning(const char *fmt, ...)
> {
> @@ -226,11 +276,14 @@ 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') {
> sym->def[def].tri = mod;
> +
> +
> sym->flags |= def_flags;
> break;
> }
> @@ -246,9 +299,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 +332,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;
> @@ -545,6 +608,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)) {
> @@ -576,6 +640,32 @@ int conf_read(const char *name)
> continue;
> conf_unsaved++;
> /* maybe print value in verbose mode... */
> + if (verbose) {
> + switch (sym->type) {
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + if (sym->def[S_DEF_USER].tri != sym->curr.tri) {
> + if (sym->dir_dep.tri < sym->def[S_DEF_USER].tri)
> + conf_error_log(DIR_DEP, sym,
> + " due to unmet direct dependencies\n",
> + NULL);
> + if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri)
> + conf_error_log(REV_DEP, sym,
> + " due to it is selected\n", NULL);
> + }
> + break;
> + case S_INT:
> + case S_HEX:
> + case S_STRING:
> + if (sym->dir_dep.tri == no &&
> + strcmp((char *)(sym->def[S_DEF_USER].val), "") != 0)
> + conf_error_log(DIR_DEP, sym,
> + " due to unmet direct dependencies\n", NULL);
> + break;
> + default:
> + break;
> + }
> + }
> }
>
> for_all_symbols(i, sym) {
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index 471a59acecec..242b24650f47 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -38,10 +38,17 @@ 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;
> +enum error_type {
> + DIR_DEP,
> + REV_DEP,
> + RANGE
> +};
>
> /* confdata.c */
> const char *conf_get_configname(void);
> void set_all_choice_values(struct symbol *csym);
> +void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...);
>
> /* confdata.c and expr.c */
> static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 0572330bf8a7..a78f7eb64f40 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -600,7 +600,7 @@ 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;
>
> switch (sym->type) {
> case S_STRING:
> @@ -612,8 +612,15 @@ 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)
> + conf_error_log(RANGE, sym,
> + " symbol value is %lld, the range is (%lld %lld)\n",
> + val, left, right);
> + return false;
> case S_HEX:
> if (!sym_string_valid(sym, str))
> return false;
> @@ -621,8 +628,15 @@ 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)
> + conf_error_log(RANGE, sym,
> + " symbol value is 0x%llx, the range is (0x%llx 0x%llx)\n",
> + val, left, right);
> + return false;
> case S_BOOLEAN:
> case S_TRISTATE:
> switch (str[0]) {
> --
> 2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2 -next] kconfig: add dependency warning print about invalid values in verbose mode
2023-11-02 2:50 ` 郭思远
@ 2023-11-06 3:51 ` Masahiro Yamada
0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2023-11-06 3:51 UTC (permalink / raw)
To: 郭思远
Cc: linux-kbuild, linux-kernel, senozhatsky, mr.bossman075, pengpeng,
sunying
On Thu, Nov 2, 2023 at 4:51 AM 郭思远 <guosy@buaa.edu.cn> wrote:
>
> I am writing to inquire about the status of my patch submission.
>
> Are there any issues with this patch that we need to modify?
Yes, many issues.
This patch will break the user interface
because printing messages to stderr
is not always allowed.
For example,
$ make nconfig KCONFIG_VERBOSE=1
Go to
Kernel hacking --->
printk and dmesg options --->
Default console loglevel (1-15) -->
Input 20
The error message will mess up the ncurses display.
You need to use conf_warning().
You introduced memory leak because you called
str_new(), but did not call str_free().
For the environment variable, please use
something specific and descriptive.
For example, KCONFIG_WARN_CHANGED_INPUT
Lastly, please do not scatter the code everywhere.
I guess the right place to inject the checker is
conf_write() (and conf_write_defconfig() as well).
} else if (!(sym->flags & SYMBOL_CHOICE) &&
!(sym->flags & SYMBOL_WRITTEN)) {
sym_calc_value(sym);
if (warn_changed_input) <--
warn_changed_user_input(sym); <--
if (!(sym->flags & SYMBOL_WRITE))
goto next;
>
> > On Fri, Sep 5, 2023 at 5:45PM Ying Sun <sunying@nj.iscas.ac.cn>wrote:
> >
> > 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>
> > ---
> > v1 -> v2:
> > * Reduced the number of code lines by refactoring and simplifying the logic.
> > * Changed the print "ERROR" to "WARNING".
> > * Focused on handling dependency errors: dir_dep and rev_dep, and range error.
> > - A downgrade from 'y' to 'm' has be warned.
> > - A new CONFIG option should not be warned.
> > - Overwriting caused by default value is not an error and is no longer printed.
> > * Fixed style issues.
> > ---
> > scripts/kconfig/confdata.c | 100 +++++++++++++++++++++++++++++++++++--
> > scripts/kconfig/lkc.h | 7 +++
> > scripts/kconfig/symbol.c | 24 +++++++--
> > 3 files changed, 121 insertions(+), 10 deletions(-)
> >
> > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> > index 4a6811d77d18..86794ab39d7d 100644
> > --- a/scripts/kconfig/confdata.c
> > +++ b/scripts/kconfig/confdata.c
> > @@ -154,6 +154,56 @@ static void conf_message(const char *fmt, ...)
> >
> > static const char *conf_filename;
> > static int conf_lineno, conf_warnings;
> > +const char *verbose;
> > +
> > +void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...)
> > +{
> > + static char *const tristate2str[3] = {"n", "m", "y"};
> > + struct gstr gs = str_new();
> > + char s[100];
> > + char *oldval = NULL;
> > + va_list args;
> > +
> > + va_start(args, log);
> > + vsnprintf(s, sizeof(s), log, args);
> > + va_end(args);
> > +
> > + switch (sym->type) {
> > + case S_BOOLEAN:
> > + case S_TRISTATE:
> > + oldval = tristate2str[sym->def[S_DEF_USER].tri];
> > + break;
> > + case S_INT:
> > + case S_HEX:
> > + case S_STRING:
> > + oldval = sym->def[S_DEF_USER].val;
> > + default:
> > + break;
> > + }
> > +
> > + str_printf(&gs,
> > + "\nWARNING : %s [%s] value is invalid\n",
> > + sym->name, oldval);
> > + str_printf(&gs, s);
> > + switch (type) {
> > + case DIR_DEP:
> > + 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");
> > + break;
> > + case REV_DEP:
> > + 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");
> > + break;
> > + default:
> > + break;
> > + }
> > + fputs(str_get(&gs), stderr);
> > +}
> >
> > static void conf_warning(const char *fmt, ...)
> > {
> > @@ -226,11 +276,14 @@ 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') {
> > sym->def[def].tri = mod;
> > +
> > +
> > sym->flags |= def_flags;
> > break;
> > }
> > @@ -246,9 +299,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 +332,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;
> > @@ -545,6 +608,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)) {
> > @@ -576,6 +640,32 @@ int conf_read(const char *name)
> > continue;
> > conf_unsaved++;
> > /* maybe print value in verbose mode... */
> > + if (verbose) {
> > + switch (sym->type) {
> > + case S_BOOLEAN:
> > + case S_TRISTATE:
> > + if (sym->def[S_DEF_USER].tri != sym->curr.tri) {
> > + if (sym->dir_dep.tri < sym->def[S_DEF_USER].tri)
> > + conf_error_log(DIR_DEP, sym,
> > + " due to unmet direct dependencies\n",
> > + NULL);
> > + if (sym->rev_dep.tri > sym->def[S_DEF_USER].tri)
> > + conf_error_log(REV_DEP, sym,
> > + " due to it is selected\n", NULL);
> > + }
> > + break;
> > + case S_INT:
> > + case S_HEX:
> > + case S_STRING:
> > + if (sym->dir_dep.tri == no &&
> > + strcmp((char *)(sym->def[S_DEF_USER].val), "") != 0)
> > + conf_error_log(DIR_DEP, sym,
> > + " due to unmet direct dependencies\n", NULL);
> > + break;
> > + default:
> > + break;
> > + }
> > + }
> > }
> >
> > for_all_symbols(i, sym) {
> > diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> > index 471a59acecec..242b24650f47 100644
> > --- a/scripts/kconfig/lkc.h
> > +++ b/scripts/kconfig/lkc.h
> > @@ -38,10 +38,17 @@ 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;
> > +enum error_type {
> > + DIR_DEP,
> > + REV_DEP,
> > + RANGE
> > +};
> >
> > /* confdata.c */
> > const char *conf_get_configname(void);
> > void set_all_choice_values(struct symbol *csym);
> > +void conf_error_log(enum error_type type, struct symbol *sym, char *log, ...);
> >
> > /* confdata.c and expr.c */
> > static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
> > diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> > index 0572330bf8a7..a78f7eb64f40 100644
> > --- a/scripts/kconfig/symbol.c
> > +++ b/scripts/kconfig/symbol.c
> > @@ -600,7 +600,7 @@ 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;
> >
> > switch (sym->type) {
> > case S_STRING:
> > @@ -612,8 +612,15 @@ 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)
> > + conf_error_log(RANGE, sym,
> > + " symbol value is %lld, the range is (%lld %lld)\n",
> > + val, left, right);
> > + return false;
> > case S_HEX:
> > if (!sym_string_valid(sym, str))
> > return false;
> > @@ -621,8 +628,15 @@ 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)
> > + conf_error_log(RANGE, sym,
> > + " symbol value is 0x%llx, the range is (0x%llx 0x%llx)\n",
> > + val, left, right);
> > + return false;
> > case S_BOOLEAN:
> > case S_TRISTATE:
> > switch (str[0]) {
> > --
> > 2.17.1
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-11-06 3:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 0:24 [PATCH] kconfig: add dependency warning print about invalid values in verbose mode sunying
2023-08-19 23:01 ` Masahiro Yamada
2023-08-20 2:40 ` Jesse T
2023-08-23 1:55 ` Sergey Senozhatsky
2023-08-23 21:52 ` Masahiro Yamada
2023-08-24 0:47 ` Sergey Senozhatsky
2023-08-29 7:49 ` 郭思远
2023-09-05 9:43 ` [PATCHv2 -next] " sunying
2023-11-02 2:50 ` 郭思远
2023-11-06 3:51 ` 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).