| Message ID | 20210215122513.1773897-3-mic@digikod.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Kconfig oldconfig string update | expand |
Mickaël Salaün <mic@digikod.net> writes: > Content of string configuration may depend on related kernel > configurations. Modify oldconfig and syncconfig to inform users about > possible required configuration update and give them the opportunity to > update it: > * if dependencies of this string has changed (e.g. enabled or disabled), > * and if the current value of this string is different than the (new) > default one. I have a number of questions: 1. Why is a change in dependencies necessarily means that the dependent's value must be revised? Here is a specific example (to make sure we are talking about the same things): config FOO string "Foo value" depends on BAR || BAZ Why, in the general case, when I disable BAR and enable BAZ I must also revise the value of FOO? 2. How do you know that what's in the user's .config is the old default and in Kconfig -- the new default value? What if in the user's .config is a custom value (with which the user is perfectly happy) and what's in Kconfig is the old default (which the user has already seen)? 3. Why limit this to strings only? > This is particularly relevant for CONFIG_LSM which contains a list of > LSMs enabled at boot, but users will not have a chance to update this > list with a make oldconfig. If my understanding above is correct, this feels like it's been purpose- made to address whatever issue you are having with CONFIG_LSM. If so, what about potential numerous other options that don't have this issue but will now be presented to the user for modification?
On 15/02/2021 15:13, Boris Kolpackov wrote: > Mickaël Salaün <mic@digikod.net> writes: > >> Content of string configuration may depend on related kernel >> configurations. Modify oldconfig and syncconfig to inform users about >> possible required configuration update and give them the opportunity to >> update it: >> * if dependencies of this string has changed (e.g. enabled or disabled), >> * and if the current value of this string is different than the (new) >> default one. > > I have a number of questions: > > 1. Why is a change in dependencies necessarily means that the dependent's > value must be revised? Here is a specific example (to make sure we are > talking about the same things): > > config FOO > string "Foo value" > depends on BAR || BAZ > > Why, in the general case, when I disable BAR and enable BAZ I must > also revise the value of FOO? It may be necessary, or not, depending of the use of the string. This semantic is not clearly expressed by kconfig but looking at the current configuration, there is only 4 strings depending on more than one dependency: * SIMDISK1_FILENAME for arch/xtensa * CMDLINE for arch/sh * SECURITY_TOMOYO_POLICY_LOADER * SECURITY_TOMOYO_ACTIVATION_TRIGGER Such patterns seem in line with this patch. > > 2. How do you know that what's in the user's .config is the old default > and in Kconfig -- the new default value? What if in the user's .config > is a custom value (with which the user is perfectly happy) and what's > in Kconfig is the old default (which the user has already seen)? The current behavior (i.e. keeping the current user config) is not changed. The oldconfig target only stops when a string may require an update, shows to the user the (potentially new but not necessary best) default value along with the value already in place in the .config file, and if the user just type enter this current value will not be changed. > > 3. Why limit this to strings only? Strings contain configuration blobs that may be interpreted by the kernel but not by kconfig (cf. CONFIG_LSM). It will still be possible to handle other types if there is some related use cases. > > >> This is particularly relevant for CONFIG_LSM which contains a list of >> LSMs enabled at boot, but users will not have a chance to update this >> list with a make oldconfig. > > If my understanding above is correct, this feels like it's been purpose- > made to address whatever issue you are having with CONFIG_LSM. If so, > what about potential numerous other options that don't have this issue > but will now be presented to the user for modification? This patch series helps address the LSM stacking issue. The 4 other cases may benefit from this patch too.
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 18a233d27a8d..8633dacd39a9 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) printf("%s", str); } +static bool may_need_string_update(struct symbol *sym, const char *def) +{ + const struct symbol *dep_sym; + const struct expr *e; + + if (sym->type != S_STRING) + return false; + if (strcmp(def, sym_get_string_default(sym)) == 0) + return false; + /* + * The user may want to synchronize the content of a string related to + * changed dependencies (e.g. CONFIG_LSM). + */ + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { + if (dep_sym->flags & SYMBOL_CHANGED) + return true; + } + return false; +} + static int conf_askvalue(struct symbol *sym, const char *def) { enum symbol_type type = sym_get_type(sym); @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) switch (input_mode) { case oldconfig: case syncconfig: - if (sym_has_value(sym)) { + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { printf("%s\n", def); return 0; } @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) printf("%*s%s ", indent - 1, "", menu->prompt->text); printf("(%s) ", sym->name); def = sym_get_string_value(sym); - if (def) - printf("[%s] ", def); + if (def) { + if (may_need_string_update(sym, def)) { + indent += 2; + printf("\n%*sDefault value is [%s]\n", + indent - 1, "", + sym_get_string_default(sym)); + printf("%*sCurrent value is [%s] ", + indent - 1, "", def); + indent -= 2; + } else { + printf("[%s] ", def); + } + } if (!conf_askvalue(sym, def)) return 0; switch (line[0]) {