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@xxxxxxxxxxxxxx>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@xxxxxxxxxxx>
> Signed-off-by: Ying Sun <sunying@xxxxxxxxxxxxxx>
> ---
> 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
