From: Mickaël Salaün <[email protected]>
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.
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.
Cc: Casey Schaufler <[email protected]>
Cc: James Morris <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Serge E. Hallyn <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
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]) {
--
2.30.0
Micka?l Sala?n <[email protected]> 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 <[email protected]> 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.