After reading some of the mails about the
defconfig files I decided to take a look at kconfig.
We already decided to go for a solution where the defconfig
files contains only the minimal configuration.
Uwe developed a script to assist but having native support
in kconfig is preferable.
This patch set introduce "alldefconfig".
alldefconfig create a configuration with all values set
to default values.
This can be usefull when we check if we do have a sane
set of default values. Rhe config can be inspected
using menuconfig.
The patches also introduce "savedefconfig" which
reads the current configuration and save it
as a minimal configuration.
In my testing I benchmarked it with one
of the arm defconfings and the final result is
the same (symbols appear in different order).
The first two patches is not relevant to the above.
They was just stuff I had pending.
The patches has seen only light testing for now,
and any feedback is welcome.
After finishing off these patches I will take a closer
look at the patch form Stephen that introduce
a more advanced select.
Note: Anyone that have heard from Roman Zippel recently?
The patches:
Roman Zippel (1):
kconfig: print more info when we see a recursive dependency
Sam Ravnborg (3):
kconfig: save location of config symbols
kconfig: add alldefconfig
kconfig: add savedefconfig
Documentation/kbuild/kconfig.txt | 2 +-
scripts/kconfig/Makefile | 13 ++-
scripts/kconfig/conf.c | 46 ++++++--
scripts/kconfig/confdata.c | 94 +++++++++++++++
scripts/kconfig/expr.h | 1 +
scripts/kconfig/lkc.h | 2 +
scripts/kconfig/lkc_proto.h | 1 +
scripts/kconfig/menu.c | 2 +
scripts/kconfig/symbol.c | 242 +++++++++++++++++++++++++++++++++++---
9 files changed, 373 insertions(+), 30 deletions(-)
Sam
>From f427c3886d7916ad8c6575964a753a33507d9f2e Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 24 Jul 2010 20:17:33 +0200
Subject: [PATCH 1/4] kconfig: save location of config symbols
When we add a new config symbol save the file/line
so we later can refer to their location.
The information is saved as a property to a config symbol
because we may have multiple definitions of the same symbol.
Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/kconfig/expr.h | 1 +
scripts/kconfig/menu.c | 2 ++
scripts/kconfig/symbol.c | 2 ++
3 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 891cd9c..4526a58 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -131,6 +131,7 @@ enum prop_type {
P_SELECT, /* select BAR */
P_RANGE, /* range 7..100 (for a symbol) */
P_ENV, /* value from environment variable */
+ P_SYMBOL, /* where a symbol is defined */
};
struct property {
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 203632c..6400823 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -58,6 +58,8 @@ void menu_add_entry(struct symbol *sym)
*last_entry_ptr = menu;
last_entry_ptr = &menu->next;
current_entry = menu;
+ if (sym)
+ menu_add_prop(P_SYMBOL, NULL, NULL, NULL);
}
void menu_end_entry(void)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 2e7a048..de36f3e 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -943,6 +943,8 @@ const char *prop_get_type_name(enum prop_type type)
return "select";
case P_RANGE:
return "range";
+ case P_SYMBOL:
+ return "symbol";
case P_UNKNOWN:
break;
}
--
1.6.0.6
>From 874dbe17dd7b55238bdf995461488fc84196987c Mon Sep 17 00:00:00 2001
From: Roman Zippel <[email protected]>
Date: Wed, 31 Dec 2008 03:55:00 +0100
Subject: [PATCH 2/4] kconfig: print more info when we see a recursive dependency
Consider following kconfig file:
config TEST1
bool "test 1"
depends on TEST2
config TEST2
bool "test 2"
depends on TEST1
Previously kconfig would report:
foo:6:error: found recursive dependency: TEST2 -> TEST1 -> TEST2
With the following patch kconfig reports:
foo:5:error: recursive dependency detected!
foo:5: symbol TEST2 depends on TEST1
foo:1: symbol TEST1 depends on TEST2
Note that we now report where the offending symbols are defined.
This can be a great help for complex situations involving
several files.
Patch is originally from Roman Zippel with a few adjustments by Sam.
Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/kconfig/symbol.c | 144 ++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 128 insertions(+), 16 deletions(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index de36f3e..0ea9c46 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -765,6 +765,110 @@ struct symbol **sym_re_search(const char *pattern)
return sym_arr;
}
+/*
+ * When we check for recursive dependencies we use a stack to save
+ * current state so we can print out relevant info to user.
+ * The entries are located on the call stack so no need to free memory.
+ * Note inser() remove() must always match to properly clear the stack.
+ */
+static struct dep_stack {
+ struct dep_stack *prev, *next;
+ struct symbol *sym;
+ struct property *prop;
+ struct expr *expr;
+} *check_top;
+
+static void dep_stack_insert(struct dep_stack *stack, struct symbol *sym)
+{
+ memset(stack, 0, sizeof(*stack));
+ if (check_top)
+ check_top->next = stack;
+ stack->prev = check_top;
+ stack->sym = sym;
+ check_top = stack;
+}
+
+static void dep_stack_remove(void)
+{
+ check_top = check_top->prev;
+ if (check_top)
+ check_top->next = NULL;
+}
+
+/*
+ * Called when we have detected a recursive dependency.
+ * check_top point to the top of the stact so we use
+ * the ->prev pointer to locate the bottom of the stack.
+ */
+static void sym_check_print_recursive(struct symbol *last_sym)
+{
+ struct dep_stack *stack;
+ struct symbol *sym, *next_sym;
+ struct menu *menu = NULL;
+ struct property *prop;
+ struct dep_stack cv_stack;
+
+ if (sym_is_choice_value(last_sym)) {
+ dep_stack_insert(&cv_stack, last_sym);
+ last_sym = prop_get_symbol(sym_get_choice_prop(last_sym));
+ }
+
+ for (stack = check_top; stack != NULL; stack = stack->prev)
+ if (stack->sym == last_sym)
+ break;
+ if (!stack) {
+ fprintf(stderr, "unexpected recursive dependency error\n");
+ return;
+ }
+
+ for (; stack; stack = stack->next) {
+ sym = stack->sym;
+ next_sym = stack->next ? stack->next->sym : last_sym;
+ prop = stack->prop;
+
+ /* for choice values find the menu entry (used below) */
+ if (sym_is_choice(sym) || sym_is_choice_value(sym)) {
+ for (prop = sym->prop; prop; prop = prop->next) {
+ menu = prop->menu;
+ if (prop->menu)
+ break;
+ }
+ }
+ if (stack->sym == last_sym)
+ fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
+ prop->file->name, prop->lineno);
+ if (stack->expr) {
+ fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
+ prop->file->name, prop->lineno,
+ sym->name ? sym->name : "<choice>",
+ prop_get_type_name(prop->type),
+ next_sym->name ? next_sym->name : "<choice>");
+ } else if (stack->prop) {
+ fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
+ prop->file->name, prop->lineno,
+ sym->name ? sym->name : "<choice>",
+ next_sym->name ? next_sym->name : "<choice>");
+ } else if (sym_is_choice(sym)) {
+ fprintf(stderr, "%s:%d:\tchoice %s contains symbol %s\n",
+ menu->file->name, menu->lineno,
+ sym->name ? sym->name : "<choice>",
+ next_sym->name ? next_sym->name : "<choice>");
+ } else if (sym_is_choice_value(sym)) {
+ fprintf(stderr, "%s:%d:\tsymbol %s is part of choice %s\n",
+ menu->file->name, menu->lineno,
+ sym->name ? sym->name : "<choice>",
+ next_sym->name ? next_sym->name : "<choice>");
+ } else {
+ fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
+ prop->file->name, prop->lineno,
+ sym->name ? sym->name : "<choice>",
+ next_sym->name ? next_sym->name : "<choice>");
+ }
+ }
+
+ if (check_top == &cv_stack)
+ dep_stack_remove();
+}
static struct symbol *sym_check_expr_deps(struct expr *e)
{
@@ -792,7 +896,7 @@ static struct symbol *sym_check_expr_deps(struct expr *e)
default:
break;
}
- printf("Oops! How to check %d?\n", e->type);
+ fprintf(stderr, "Oops! How to check %d?\n", e->type);
return NULL;
}
@@ -801,24 +905,33 @@ static struct symbol *sym_check_sym_deps(struct symbol *sym)
{
struct symbol *sym2;
struct property *prop;
+ struct dep_stack stack;
+
+ dep_stack_insert(&stack, sym);
sym2 = sym_check_expr_deps(sym->rev_dep.expr);
if (sym2)
- return sym2;
+ goto out;
for (prop = sym->prop; prop; prop = prop->next) {
if (prop->type == P_CHOICE || prop->type == P_SELECT)
continue;
+ stack.prop = prop;
sym2 = sym_check_expr_deps(prop->visible.expr);
if (sym2)
break;
if (prop->type != P_DEFAULT || sym_is_choice(sym))
continue;
+ stack.expr = prop->expr;
sym2 = sym_check_expr_deps(prop->expr);
if (sym2)
break;
+ stack.expr = NULL;
}
+out:
+ dep_stack_remove();
+
return sym2;
}
@@ -827,6 +940,9 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
struct symbol *sym, *sym2;
struct property *prop;
struct expr *e;
+ struct dep_stack stack;
+
+ dep_stack_insert(&stack, choice);
prop = sym_get_choice_prop(choice);
expr_list_for_each_sym(prop->expr, e, sym)
@@ -840,10 +956,8 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
expr_list_for_each_sym(prop->expr, e, sym) {
sym2 = sym_check_sym_deps(sym);
- if (sym2) {
- fprintf(stderr, " -> %s", sym->name);
+ if (sym2)
break;
- }
}
out:
expr_list_for_each_sym(prop->expr, e, sym)
@@ -853,6 +967,8 @@ out:
prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
sym2 = choice;
+ dep_stack_remove();
+
return sym2;
}
@@ -862,18 +978,20 @@ struct symbol *sym_check_deps(struct symbol *sym)
struct property *prop;
if (sym->flags & SYMBOL_CHECK) {
- fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
- sym->prop->file->name, sym->prop->lineno,
- sym->name ? sym->name : "<choice>");
+ sym_check_print_recursive(sym);
return sym;
}
if (sym->flags & SYMBOL_CHECKED)
return NULL;
if (sym_is_choice_value(sym)) {
+ struct dep_stack stack;
+
/* for choice groups start the check with main choice symbol */
+ dep_stack_insert(&stack, sym);
prop = sym_get_choice_prop(sym);
sym2 = sym_check_deps(prop_get_symbol(prop));
+ dep_stack_remove();
} else if (sym_is_choice(sym)) {
sym2 = sym_check_choice_deps(sym);
} else {
@@ -882,14 +1000,8 @@ struct symbol *sym_check_deps(struct symbol *sym)
sym->flags &= ~SYMBOL_CHECK;
}
- if (sym2) {
- fprintf(stderr, " -> %s", sym->name ? sym->name : "<choice>");
- if (sym2 == sym) {
- fprintf(stderr, "\n");
- zconfnerrs++;
- sym2 = NULL;
- }
- }
+ if (sym2 && sym2 == sym)
+ sym2 = NULL;
return sym2;
}
--
1.6.0.6
>From e797380842b48477cc9d5e625420c3426caff6e2 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 24 Jul 2010 23:51:23 +0200
Subject: [PATCH 3/4] kconfig: add alldefconfig
alldefconfig create a configuration with all values set
to their default value (form the Kconfig files).
This may be usefull when we try to use more sensible default
values and may also be used in combination with
the minimal defconfigs.
Signed-off-by: Sam Ravnborg <[email protected]>
---
Documentation/kbuild/kconfig.txt | 2 +-
scripts/kconfig/Makefile | 8 ++++++--
scripts/kconfig/conf.c | 28 ++++++++++++++++++----------
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
index b2cb16e..cca46b1 100644
--- a/Documentation/kbuild/kconfig.txt
+++ b/Documentation/kbuild/kconfig.txt
@@ -65,7 +65,7 @@ also use the environment variable KCONFIG_ALLCONFIG as a flag or a
filename that contains config symbols that the user requires to be
set to a specific value. If KCONFIG_ALLCONFIG is used without a
filename, "make *config" checks for a file named
-"all{yes/mod/no/random}.config" (corresponding to the *config command
+"all{yes/mod/no/def/random}.config" (corresponding to the *config command
that was used) for symbol values that are to be forced. If this file
is not found, it checks for a file named "all.config" to contain forced
values.
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 7ea649d..366711a 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -95,7 +95,7 @@ update-po-config: $(obj)/kxgettext $(obj)/gconf.glade.h
$(Q)rm -f arch/um/Kconfig.arch
$(Q)rm -f $(obj)/config.pot
-PHONY += randconfig allyesconfig allnoconfig allmodconfig defconfig
+PHONY += randconfig allyesconfig allnoconfig allmodconfig alldefconfig defconfig
randconfig: $(obj)/conf
$< -r $(Kconfig)
@@ -109,6 +109,9 @@ allnoconfig: $(obj)/conf
allmodconfig: $(obj)/conf
$< -m $(Kconfig)
+alldefconfig: $(obj)/conf
+ $< -f $(Kconfig)
+
defconfig: $(obj)/conf
ifeq ($(KBUILD_DEFCONFIG),)
$< -d $(Kconfig)
@@ -132,7 +135,8 @@ help:
@echo ' localyesconfig - Update current config converting local mods to core'
@echo ' silentoldconfig - Same as oldconfig, but quietly, additionally update deps'
@echo ' randconfig - New config with random answer to all options'
- @echo ' defconfig - New config with default answer to all options'
+ @echo ' defconfig - New config with default from ARCH supplied defconfig'
+ @echo ' alldefconfig - New config with all symbols set to default'
@echo ' allmodconfig - New config selecting modules when possible'
@echo ' allyesconfig - New config where all options are accepted with yes'
@echo ' allnoconfig - New config where all options are answered with no'
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 9960d1c..2b4775e 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -23,10 +23,11 @@ enum {
ask_all,
ask_new,
ask_silent,
- set_default,
+ read_default,
set_yes,
set_mod,
set_no,
+ set_default,
set_random
} input_mode = ask_all;
char *defconfig_file;
@@ -439,7 +440,7 @@ int main(int ac, char **av)
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((opt = getopt(ac, av, "osdD:nmyrh")) != -1) {
+ while ((opt = getopt(ac, av, "osdD:nmyfrh")) != -1) {
switch (opt) {
case 'o':
input_mode = ask_silent;
@@ -449,10 +450,10 @@ int main(int ac, char **av)
sync_kconfig = 1;
break;
case 'd':
- input_mode = set_default;
+ input_mode = read_default;
break;
case 'D':
- input_mode = set_default;
+ input_mode = read_default;
defconfig_file = optarg;
break;
case 'n':
@@ -464,6 +465,9 @@ int main(int ac, char **av)
case 'y':
input_mode = set_yes;
break;
+ case 'f':
+ input_mode = set_default;
+ break;
case 'r':
{
struct timeval now;
@@ -512,7 +516,7 @@ int main(int ac, char **av)
}
switch (input_mode) {
- case set_default:
+ case read_default:
if (!defconfig_file)
defconfig_file = conf_get_default_confname();
if (conf_read(defconfig_file)) {
@@ -537,10 +541,11 @@ int main(int ac, char **av)
break;
}
switch (input_mode) {
- case set_no: name = "allno.config"; break;
- case set_mod: name = "allmod.config"; break;
- case set_yes: name = "allyes.config"; break;
- case set_random: name = "allrandom.config"; break;
+ case set_no: name = "allno.config"; break;
+ case set_mod: name = "allmod.config"; break;
+ case set_yes: name = "allyes.config"; break;
+ case set_default: name = "alldefault.config"; break;
+ case set_random: name = "allrandom.config"; break;
default: break;
}
if (!stat(name, &tmpstat))
@@ -574,10 +579,13 @@ int main(int ac, char **av)
case set_mod:
conf_set_all_new_symbols(def_mod);
break;
+ case set_default:
+ conf_set_all_new_symbols(def_default);
+ break;
case set_random:
conf_set_all_new_symbols(def_random);
break;
- case set_default:
+ case read_default:
conf_set_all_new_symbols(def_default);
break;
case ask_new:
--
1.6.0.6
>From 5edffcc6890a7dbd43b8da9c453bee794e81e7c7 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sun, 25 Jul 2010 23:18:47 +0200
Subject: [PATCH 4/4] kconfig: add savedefconfig
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
savedefconfig is used to save a minimal configuration
that is useful for a defconfig.
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: Uwe Kleine-König <[email protected]>
Cc: Stephen Rothwell <[email protected]>
---
scripts/kconfig/Makefile | 7 +++-
scripts/kconfig/conf.c | 20 ++++++++-
scripts/kconfig/confdata.c | 94 ++++++++++++++++++++++++++++++++++++++++++
scripts/kconfig/lkc.h | 2 +
scripts/kconfig/lkc_proto.h | 1 +
scripts/kconfig/symbol.c | 96 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 217 insertions(+), 3 deletions(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 366711a..aeb04c2 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -95,7 +95,8 @@ update-po-config: $(obj)/kxgettext $(obj)/gconf.glade.h
$(Q)rm -f arch/um/Kconfig.arch
$(Q)rm -f $(obj)/config.pot
-PHONY += randconfig allyesconfig allnoconfig allmodconfig alldefconfig defconfig
+PHONY += randconfig allyesconfig allnoconfig allmodconfig alldefconfig
+PHONY += defconfig savedefconfig
randconfig: $(obj)/conf
$< -r $(Kconfig)
@@ -112,6 +113,9 @@ allmodconfig: $(obj)/conf
alldefconfig: $(obj)/conf
$< -f $(Kconfig)
+savedefconfig: $(obj)/conf
+ $< -M defconfig $(Kconfig)
+
defconfig: $(obj)/conf
ifeq ($(KBUILD_DEFCONFIG),)
$< -d $(Kconfig)
@@ -140,6 +144,7 @@ help:
@echo ' allmodconfig - New config selecting modules when possible'
@echo ' allyesconfig - New config where all options are accepted with yes'
@echo ' allnoconfig - New config where all options are answered with no'
+ @echo ' savedefconfig - Save current config as ./defconfig (minimal config)'
# lxdialog stuff
check-lxdialog := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 2b4775e..b1a903b 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -28,7 +28,8 @@ enum {
set_mod,
set_no,
set_default,
- set_random
+ set_random,
+ save_defconfig,
} input_mode = ask_all;
char *defconfig_file;
@@ -440,7 +441,7 @@ int main(int ac, char **av)
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((opt = getopt(ac, av, "osdD:nmyfrh")) != -1) {
+ while ((opt = getopt(ac, av, "osdD:nmM:yfrh")) != -1) {
switch (opt) {
case 'o':
input_mode = ask_silent;
@@ -485,6 +486,10 @@ int main(int ac, char **av)
input_mode = set_random;
break;
}
+ case 'M':
+ input_mode = save_defconfig;
+ defconfig_file = optarg;
+ break;
case 'h':
printf(_("See README for usage info\n"));
exit(0);
@@ -553,6 +558,9 @@ int main(int ac, char **av)
else if (!stat("all.config", &tmpstat))
conf_read_simple("all.config", S_DEF_USER);
break;
+ case save_defconfig:
+ conf_read(NULL);
+ break;
default:
break;
}
@@ -601,6 +609,8 @@ int main(int ac, char **av)
check_conf(&rootmenu);
} while (conf_cnt);
break;
+ case save_defconfig:
+ break;
}
if (sync_kconfig) {
@@ -615,6 +625,12 @@ int main(int ac, char **av)
fprintf(stderr, _("\n*** Error during update of the kernel configuration.\n\n"));
return 1;
}
+ } else if (input_mode == save_defconfig) {
+ if (conf_write_defconfig(defconfig_file)) {
+ fprintf(stderr, _("\n*** Error during writing of mini config to %s.\n\n"),
+ defconfig_file);
+ return 1;
+ }
} else {
if (conf_write(NULL)) {
fprintf(stderr, _("\n*** Error during writing of the kernel configuration.\n\n"));
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index c4dec80..76c4e23 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -396,6 +396,100 @@ int conf_read(const char *name)
return 0;
}
+/*
+ * Write out a minimal config.
+ * All values that has default values are skipped as this is redundant.
+ */
+int conf_write_defconfig(const char *filename)
+{
+ struct symbol *sym;
+ const char *str;
+ FILE *out;
+ int i, l;
+
+ out = fopen(filename, "w");
+ if (!out)
+ return 1;
+
+ sym_clear_all_valid();
+
+ for_all_symbols(i, sym) {
+ sym_calc_value(sym);
+ if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
+ goto next_symbol;
+ /* If we cannot change the symbol - skip */
+ if (!sym_is_changable(sym))
+ goto next_symbol;
+ /* If symbol equals to default value - skip */
+ if (strcmp(sym_get_string_value(sym), sym_get_string_default(sym)) == 0)
+ goto next_symbol;
+
+ /* choice symbols does not have a value - skip */
+ if (sym_is_choice(sym))
+ goto next_symbol;
+ /*
+ * If symbol is a choice value and equals to the
+ * default for a choice - skip.
+ * But only if value equal to "y".
+ */
+ if (sym_is_choice_value(sym)) {
+ struct symbol *cs;
+ struct symbol *ds;
+
+ cs = prop_get_symbol(sym_get_choice_prop(sym));
+ ds = sym_choice_default(cs);
+ if (sym == ds) {
+ if ((sym->type == S_BOOLEAN ||
+ sym->type == S_TRISTATE) &&
+ sym_get_tristate_value(sym) == yes)
+ goto next_symbol;
+ }
+ }
+ switch (sym->type) {
+ case S_BOOLEAN:
+ case S_TRISTATE:
+ switch (sym_get_tristate_value(sym)) {
+ case no:
+ fprintf(out, "# CONFIG_%s is not set\n", sym->name);
+ break;
+ case yes:
+ fprintf(out, "CONFIG_%s=y\n", sym->name);
+ break;
+ case mod:
+ fprintf(out, "CONFIG_%s=m\n", sym->name);
+ break;
+ }
+ break;
+ case S_STRING:
+ str = sym_get_string_value(sym);
+ fprintf(out, "CONFIG_%s=\"", sym->name);
+ while (1) {
+ l = strcspn(str, "\"\\");
+ if (l) {
+ fwrite(str, l, 1, out);
+ str += l;
+ }
+ if (!*str)
+ break;
+ fprintf(out, "\\%c", *str++);
+ }
+ fputs("\"\n", out);
+ break;
+ case S_HEX:
+ case S_INT:
+ str = sym_get_string_value(sym);
+ fprintf(out, "CONFIG_%s=%s\n", sym->name, str);
+ break;
+ default:
+ break;
+ }
+next_symbol:
+ ;
+ }
+ fclose(out);
+ return 0;
+}
+
int conf_write(const char *name)
{
FILE *out;
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index ce6549c..76db065 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -126,6 +126,8 @@ void sym_init(void);
void sym_clear_all_valid(void);
void sym_set_all_changed(void);
void sym_set_changed(struct symbol *sym);
+struct symbol *sym_choice_default(struct symbol *sym);
+const char *sym_get_string_default(struct symbol *sym);
struct symbol *sym_check_deps(struct symbol *sym);
struct property *prop_alloc(enum prop_type type, struct symbol *sym);
struct symbol *prop_get_symbol(struct property *prop);
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 7cadcad..e30d5f2 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -4,6 +4,7 @@ P(conf_parse,void,(const char *name));
P(conf_read,int,(const char *name));
P(conf_read_simple,int,(const char *name, int));
P(conf_write,int,(const char *name));
+P(conf_write_defconfig,int,(const char *name));
P(conf_write_autoconf,int,(void));
P(conf_get_changed,bool,(void));
P(conf_set_changed_callback, void,(void (*fn)(void)));
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 0ea9c46..0bda646 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -216,6 +216,41 @@ static void sym_calc_visibility(struct symbol *sym)
}
}
+/*
+ * Find the default isymbol for a choice.
+ * First try the default values for the choice symbol
+ * Next locate the first visible choice value
+ * Return NULL if none was found
+ */
+struct symbol *sym_choice_default(struct symbol *sym)
+{
+ struct property *prop;
+ struct symbol *def_sym;
+ struct expr *e;
+
+ /* any of the defaults visible? */
+ for_all_defaults(sym, prop) {
+ prop->visible.tri = expr_calc_value(prop->visible.expr);
+ if (prop->visible.tri == no)
+ continue;
+ def_sym = prop_get_symbol(prop);
+ sym_calc_visibility(def_sym);
+ if (def_sym->visible != no)
+ return def_sym;
+ }
+
+ /* just get the first visible value */
+ prop = sym_get_choice_prop(sym);
+ expr_list_for_each_sym(prop->expr, e, def_sym) {
+ sym_calc_visibility(def_sym);
+ if (def_sym->visible != no)
+ return def_sym;
+ }
+
+ /* failed to locate any defaults */
+ return NULL;
+}
+
static struct symbol *sym_calc_choice(struct symbol *sym)
{
struct symbol *def_sym;
@@ -646,6 +681,67 @@ const char *sym_get_string_value(struct symbol *sym)
return (const char *)sym->curr.val;
}
+/*
+ * Find the default value associated to a symbol.
+ * For tristate symbol hande the modules=n case
+ * in which case "m" bocomes "y".
+ * If the symbol does not have any default fallback
+ * to the fixed default values.
+ */
+const char *sym_get_string_default(struct symbol *sym)
+{
+ struct property *prop;
+ struct symbol *ds;
+ tristate val;
+
+ prop = sym_get_default_prop(sym);
+ if (!prop)
+ goto default_value;
+ ds = prop_get_symbol(prop);
+ if (!ds)
+ goto default_value;
+
+ switch (sym->type) {
+ case S_BOOLEAN:
+ case S_TRISTATE:
+ val = sym_get_tristate_value(ds);
+ switch (val) {
+ case no:
+ return "n";
+ case yes:
+ return "y";
+ case mod:
+ if (sym_is_choice_value(sym))
+ return "m";
+ sym_calc_value(modules_sym);
+ if (modules_sym->curr.tri == no)
+ return "y";
+ else
+ return "m";
+ }
+ break;
+ default:
+ return (const char *)ds->curr.val;
+ }
+
+default_value:
+
+ switch (sym->type) {
+ case S_BOOLEAN:
+ case S_TRISTATE:
+ return "n";
+ case S_INT:
+ case S_HEX:
+ return "0";
+ case S_STRING:
+ return "";
+ case S_OTHER:
+ case S_UNKNOWN:
+ break;
+ }
+ return "";
+}
+
bool sym_is_changable(struct symbol *sym)
{
return sym->visible > sym->rev_dep.tri;
--
1.6.0.6
> in kconfig is preferable.
>
> This patch set introduce "alldefconfig".
> alldefconfig create a configuration with all values set
> to default values.
> This can be usefull when we check if we do have a sane
> set of default values. Rhe config can be inspected
> using menuconfig.
>
> The patches also introduce "savedefconfig" which
> reads the current configuration and save it
> as a minimal configuration.
A funny detail that I used in my testing.
If we create a config based solely on default values like this:
make alldefconfig
then when we later save the minimal config like this:
make savedefconfig
then the resulting defconfig file is empty.
This is as expected because all symbols has default values
so there is no need to save the value of any symbol.
Sam
On 25.7.2010 23:40, Sam Ravnborg wrote:
> From 5edffcc6890a7dbd43b8da9c453bee794e81e7c7 Mon Sep 17 00:00:00 2001
> From: Sam Ravnborg <[email protected]>
> Date: Sun, 25 Jul 2010 23:18:47 +0200
> Subject: [PATCH 4/4] kconfig: add savedefconfig
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: 8bit
>
> savedefconfig is used to save a minimal configuration
> that is useful for a defconfig.
General comment: I like this more than the "replace defconfigs with
Kconfig files" approach -- the .config format is efficient and easy to
read, we have tools to edit it (make *config), the issue was the size of
the defconfigs and the updates to them, which your patches address now.
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: Uwe Kleine-König <[email protected]>
> Cc: Stephen Rothwell <[email protected]>
> ---
> scripts/kconfig/Makefile | 7 +++-
> scripts/kconfig/conf.c | 20 ++++++++-
> scripts/kconfig/confdata.c | 94 ++++++++++++++++++++++++++++++++++++++++++
> scripts/kconfig/lkc.h | 2 +
> scripts/kconfig/lkc_proto.h | 1 +
> scripts/kconfig/symbol.c | 96 +++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 217 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index 366711a..aeb04c2 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -95,7 +95,8 @@ update-po-config: $(obj)/kxgettext $(obj)/gconf.glade.h
> $(Q)rm -f arch/um/Kconfig.arch
> $(Q)rm -f $(obj)/config.pot
>
> -PHONY += randconfig allyesconfig allnoconfig allmodconfig alldefconfig defconfig
> +PHONY += randconfig allyesconfig allnoconfig allmodconfig alldefconfig
> +PHONY += defconfig savedefconfig
>
> randconfig: $(obj)/conf
> $< -r $(Kconfig)
> @@ -112,6 +113,9 @@ allmodconfig: $(obj)/conf
> alldefconfig: $(obj)/conf
> $< -f $(Kconfig)
>
> +savedefconfig: $(obj)/conf
> + $< -M defconfig $(Kconfig)
> +
Out of curiosity, do you have any mnemonics for -f and -M, or are these
just random letters that were free? :) Maybe we should add long options
like --defconfig, --allmodconfig for each of the targets and use them in
the Makefile. The meaning of the single-letter options is not obvious
sometimes. scripts/genksyms already uses getopt_long() so we would not
regress in portability.
> defconfig: $(obj)/conf
> ifeq ($(KBUILD_DEFCONFIG),)
> $< -d $(Kconfig)
> @@ -140,6 +144,7 @@ help:
> @echo ' allmodconfig - New config selecting modules when possible'
> @echo ' allyesconfig - New config where all options are accepted with yes'
> @echo ' allnoconfig - New config where all options are answered with no'
> + @echo ' savedefconfig - Save current config as ./defconfig (minimal config)'
>
> # lxdialog stuff
> check-lxdialog := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 2b4775e..b1a903b 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -28,7 +28,8 @@ enum {
> set_mod,
> set_no,
> set_default,
> - set_random
> + set_random,
> + save_defconfig,
> } input_mode = ask_all;
> char *defconfig_file;
>
> @@ -440,7 +441,7 @@ int main(int ac, char **av)
> bindtextdomain(PACKAGE, LOCALEDIR);
> textdomain(PACKAGE);
>
> - while ((opt = getopt(ac, av, "osdD:nmyfrh")) != -1) {
> + while ((opt = getopt(ac, av, "osdD:nmM:yfrh")) != -1) {
> switch (opt) {
> case 'o':
> input_mode = ask_silent;
> @@ -485,6 +486,10 @@ int main(int ac, char **av)
> input_mode = set_random;
> break;
> }
> + case 'M':
> + input_mode = save_defconfig;
> + defconfig_file = optarg;
> + break;
> case 'h':
> printf(_("See README for usage info\n"));
> exit(0);
> @@ -553,6 +558,9 @@ int main(int ac, char **av)
> else if (!stat("all.config", &tmpstat))
> conf_read_simple("all.config", S_DEF_USER);
> break;
> + case save_defconfig:
> + conf_read(NULL);
> + break;
> default:
> break;
> }
> @@ -601,6 +609,8 @@ int main(int ac, char **av)
> check_conf(&rootmenu);
> } while (conf_cnt);
> break;
> + case save_defconfig:
> + break;
> }
>
> if (sync_kconfig) {
> @@ -615,6 +625,12 @@ int main(int ac, char **av)
> fprintf(stderr, _("\n*** Error during update of the kernel configuration.\n\n"));
> return 1;
> }
> + } else if (input_mode == save_defconfig) {
> + if (conf_write_defconfig(defconfig_file)) {
> + fprintf(stderr, _("\n*** Error during writing of mini config to %s.\n\n"),
> + defconfig_file);
> + return 1;
> + }
> } else {
> if (conf_write(NULL)) {
> fprintf(stderr, _("\n*** Error during writing of the kernel configuration.\n\n"));
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index c4dec80..76c4e23 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -396,6 +396,100 @@ int conf_read(const char *name)
> return 0;
> }
>
> +/*
> + * Write out a minimal config.
> + * All values that has default values are skipped as this is redundant.
> + */
> +int conf_write_defconfig(const char *filename)
> +{
> + struct symbol *sym;
> + const char *str;
> + FILE *out;
> + int i, l;
> +
> + out = fopen(filename, "w");
> + if (!out)
> + return 1;
> +
> + sym_clear_all_valid();
> +
> + for_all_symbols(i, sym) {
> + sym_calc_value(sym);
> + if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
> + goto next_symbol;
> + /* If we cannot change the symbol - skip */
> + if (!sym_is_changable(sym))
> + goto next_symbol;
> + /* If symbol equals to default value - skip */
> + if (strcmp(sym_get_string_value(sym), sym_get_string_default(sym)) == 0)
> + goto next_symbol;
> +
> + /* choice symbols does not have a value - skip */
> + if (sym_is_choice(sym))
> + goto next_symbol;
> + /*
> + * If symbol is a choice value and equals to the
> + * default for a choice - skip.
> + * But only if value equal to "y".
> + */
> + if (sym_is_choice_value(sym)) {
> + struct symbol *cs;
> + struct symbol *ds;
> +
> + cs = prop_get_symbol(sym_get_choice_prop(sym));
> + ds = sym_choice_default(cs);
> + if (sym == ds) {
> + if ((sym->type == S_BOOLEAN ||
> + sym->type == S_TRISTATE) &&
> + sym_get_tristate_value(sym) == yes)
> + goto next_symbol;
> + }
> + }
> + switch (sym->type) {
This duplicates the switch statement in conf_write(), we should move it
into a separate function and call it from conf_write() and here.
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + switch (sym_get_tristate_value(sym)) {
> + case no:
> + fprintf(out, "# CONFIG_%s is not set\n", sym->name);
> + break;
> + case yes:
> + fprintf(out, "CONFIG_%s=y\n", sym->name);
> + break;
> + case mod:
> + fprintf(out, "CONFIG_%s=m\n", sym->name);
> + break;
> + }
> + break;
> + case S_STRING:
> + str = sym_get_string_value(sym);
> + fprintf(out, "CONFIG_%s=\"", sym->name);
> + while (1) {
> + l = strcspn(str, "\"\\");
> + if (l) {
> + fwrite(str, l, 1, out);
> + str += l;
> + }
> + if (!*str)
> + break;
> + fprintf(out, "\\%c", *str++);
> + }
> + fputs("\"\n", out);
> + break;
> + case S_HEX:
> + case S_INT:
> + str = sym_get_string_value(sym);
> + fprintf(out, "CONFIG_%s=%s\n", sym->name, str);
> + break;
> + default:
> + break;
> + }
> +next_symbol:
> + ;
> + }
The gotos should be replaced by simple continue statements if there is
nothing to do after the label.
(maybe there are more issues, I didn't look closely into it. But I like
the approach.)
Michal
> + fclose(out);
> + return 0;
> +}
> +
> int conf_write(const char *name)
> {
> FILE *out;
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index ce6549c..76db065 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -126,6 +126,8 @@ void sym_init(void);
> void sym_clear_all_valid(void);
> void sym_set_all_changed(void);
> void sym_set_changed(struct symbol *sym);
> +struct symbol *sym_choice_default(struct symbol *sym);
> +const char *sym_get_string_default(struct symbol *sym);
> struct symbol *sym_check_deps(struct symbol *sym);
> struct property *prop_alloc(enum prop_type type, struct symbol *sym);
> struct symbol *prop_get_symbol(struct property *prop);
> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
> index 7cadcad..e30d5f2 100644
> --- a/scripts/kconfig/lkc_proto.h
> +++ b/scripts/kconfig/lkc_proto.h
> @@ -4,6 +4,7 @@ P(conf_parse,void,(const char *name));
> P(conf_read,int,(const char *name));
> P(conf_read_simple,int,(const char *name, int));
> P(conf_write,int,(const char *name));
> +P(conf_write_defconfig,int,(const char *name));
> P(conf_write_autoconf,int,(void));
> P(conf_get_changed,bool,(void));
> P(conf_set_changed_callback, void,(void (*fn)(void)));
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 0ea9c46..0bda646 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -216,6 +216,41 @@ static void sym_calc_visibility(struct symbol *sym)
> }
> }
>
> +/*
> + * Find the default isymbol for a choice.
> + * First try the default values for the choice symbol
> + * Next locate the first visible choice value
> + * Return NULL if none was found
> + */
> +struct symbol *sym_choice_default(struct symbol *sym)
> +{
> + struct property *prop;
> + struct symbol *def_sym;
> + struct expr *e;
> +
> + /* any of the defaults visible? */
> + for_all_defaults(sym, prop) {
> + prop->visible.tri = expr_calc_value(prop->visible.expr);
> + if (prop->visible.tri == no)
> + continue;
> + def_sym = prop_get_symbol(prop);
> + sym_calc_visibility(def_sym);
> + if (def_sym->visible != no)
> + return def_sym;
> + }
> +
> + /* just get the first visible value */
> + prop = sym_get_choice_prop(sym);
> + expr_list_for_each_sym(prop->expr, e, def_sym) {
> + sym_calc_visibility(def_sym);
> + if (def_sym->visible != no)
> + return def_sym;
> + }
> +
> + /* failed to locate any defaults */
> + return NULL;
> +}
> +
> static struct symbol *sym_calc_choice(struct symbol *sym)
> {
> struct symbol *def_sym;
> @@ -646,6 +681,67 @@ const char *sym_get_string_value(struct symbol *sym)
> return (const char *)sym->curr.val;
> }
>
> +/*
> + * Find the default value associated to a symbol.
> + * For tristate symbol hande the modules=n case
> + * in which case "m" bocomes "y".
> + * If the symbol does not have any default fallback
> + * to the fixed default values.
> + */
> +const char *sym_get_string_default(struct symbol *sym)
> +{
> + struct property *prop;
> + struct symbol *ds;
> + tristate val;
> +
> + prop = sym_get_default_prop(sym);
> + if (!prop)
> + goto default_value;
> + ds = prop_get_symbol(prop);
> + if (!ds)
> + goto default_value;
> +
> + switch (sym->type) {
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + val = sym_get_tristate_value(ds);
> + switch (val) {
> + case no:
> + return "n";
> + case yes:
> + return "y";
> + case mod:
> + if (sym_is_choice_value(sym))
> + return "m";
> + sym_calc_value(modules_sym);
> + if (modules_sym->curr.tri == no)
> + return "y";
> + else
> + return "m";
> + }
> + break;
> + default:
> + return (const char *)ds->curr.val;
> + }
> +
> +default_value:
> +
> + switch (sym->type) {
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + return "n";
> + case S_INT:
> + case S_HEX:
> + return "0";
> + case S_STRING:
> + return "";
> + case S_OTHER:
> + case S_UNKNOWN:
> + break;
> + }
> + return "";
> +}
> +
> bool sym_is_changable(struct symbol *sym)
> {
> return sym->visible > sym->rev_dep.tri;
> >
> > randconfig: $(obj)/conf
> > $< -r $(Kconfig)
> > @@ -112,6 +113,9 @@ allmodconfig: $(obj)/conf
> > alldefconfig: $(obj)/conf
> > $< -f $(Kconfig)
> >
> > +savedefconfig: $(obj)/conf
> > + $< -M defconfig $(Kconfig)
> > +
>
> Out of curiosity, do you have any mnemonics for -f and -M, or are these
> just random letters that were free? :) Maybe we should add long options
> like --defconfig, --allmodconfig for each of the targets and use them in
> the Makefile. The meaning of the single-letter options is not obvious
> sometimes. scripts/genksyms already uses getopt_long() so we would not
> regress in portability.
Random letters that were free.
I will take a look at getopt_long() in next spin.
>
>
> > defconfig: $(obj)/conf
> > ifeq ($(KBUILD_DEFCONFIG),)
> > $< -d $(Kconfig)
> > @@ -140,6 +144,7 @@ help:
> > @echo ' allmodconfig - New config selecting modules when possible'
> > @echo ' allyesconfig - New config where all options are accepted with yes'
> > @echo ' allnoconfig - New config where all options are answered with no'
> > + @echo ' savedefconfig - Save current config as ./defconfig (minimal config)'
> >
> > # lxdialog stuff
> > check-lxdialog := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
> > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> > index 2b4775e..b1a903b 100644
> > --- a/scripts/kconfig/conf.c
> > +++ b/scripts/kconfig/conf.c
> > @@ -28,7 +28,8 @@ enum {
> > set_mod,
> > set_no,
> > set_default,
> > - set_random
> > + set_random,
> > + save_defconfig,
> > } input_mode = ask_all;
> > char *defconfig_file;
> >
> > @@ -440,7 +441,7 @@ int main(int ac, char **av)
> > bindtextdomain(PACKAGE, LOCALEDIR);
> > textdomain(PACKAGE);
> >
> > - while ((opt = getopt(ac, av, "osdD:nmyfrh")) != -1) {
> > + while ((opt = getopt(ac, av, "osdD:nmM:yfrh")) != -1) {
> > switch (opt) {
> > case 'o':
> > input_mode = ask_silent;
> > @@ -485,6 +486,10 @@ int main(int ac, char **av)
> > input_mode = set_random;
> > break;
> > }
> > + case 'M':
> > + input_mode = save_defconfig;
> > + defconfig_file = optarg;
> > + break;
> > case 'h':
> > printf(_("See README for usage info\n"));
> > exit(0);
> > @@ -553,6 +558,9 @@ int main(int ac, char **av)
> > else if (!stat("all.config", &tmpstat))
> > conf_read_simple("all.config", S_DEF_USER);
> > break;
> > + case save_defconfig:
> > + conf_read(NULL);
> > + break;
> > default:
> > break;
> > }
> > @@ -601,6 +609,8 @@ int main(int ac, char **av)
> > check_conf(&rootmenu);
> > } while (conf_cnt);
> > break;
> > + case save_defconfig:
> > + break;
> > }
> >
> > if (sync_kconfig) {
> > @@ -615,6 +625,12 @@ int main(int ac, char **av)
> > fprintf(stderr, _("\n*** Error during update of the kernel configuration.\n\n"));
> > return 1;
> > }
> > + } else if (input_mode == save_defconfig) {
> > + if (conf_write_defconfig(defconfig_file)) {
> > + fprintf(stderr, _("\n*** Error during writing of mini config to %s.\n\n"),
> > + defconfig_file);
> > + return 1;
> > + }
> > } else {
> > if (conf_write(NULL)) {
> > fprintf(stderr, _("\n*** Error during writing of the kernel configuration.\n\n"));
> > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> > index c4dec80..76c4e23 100644
> > --- a/scripts/kconfig/confdata.c
> > +++ b/scripts/kconfig/confdata.c
> > @@ -396,6 +396,100 @@ int conf_read(const char *name)
> > return 0;
> > }
> >
> > +/*
> > + * Write out a minimal config.
> > + * All values that has default values are skipped as this is redundant.
> > + */
> > +int conf_write_defconfig(const char *filename)
> > +{
> > + struct symbol *sym;
> > + const char *str;
> > + FILE *out;
> > + int i, l;
> > +
> > + out = fopen(filename, "w");
> > + if (!out)
> > + return 1;
> > +
> > + sym_clear_all_valid();
> > +
> > + for_all_symbols(i, sym) {
> > + sym_calc_value(sym);
> > + if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
> > + goto next_symbol;
> > + /* If we cannot change the symbol - skip */
> > + if (!sym_is_changable(sym))
> > + goto next_symbol;
> > + /* If symbol equals to default value - skip */
> > + if (strcmp(sym_get_string_value(sym), sym_get_string_default(sym)) == 0)
> > + goto next_symbol;
> > +
> > + /* choice symbols does not have a value - skip */
> > + if (sym_is_choice(sym))
> > + goto next_symbol;
> > + /*
> > + * If symbol is a choice value and equals to the
> > + * default for a choice - skip.
> > + * But only if value equal to "y".
> > + */
> > + if (sym_is_choice_value(sym)) {
> > + struct symbol *cs;
> > + struct symbol *ds;
> > +
> > + cs = prop_get_symbol(sym_get_choice_prop(sym));
> > + ds = sym_choice_default(cs);
> > + if (sym == ds) {
> > + if ((sym->type == S_BOOLEAN ||
> > + sym->type == S_TRISTATE) &&
> > + sym_get_tristate_value(sym) == yes)
> > + goto next_symbol;
> > + }
> > + }
> > + switch (sym->type) {
>
> This duplicates the switch statement in conf_write(), we should move it
> into a separate function and call it from conf_write() and here.
Will do.
Same goes for some of the code in symbol.c.
I will do it in a few steps to ease review.
>
>
> > + case S_BOOLEAN:
> > + case S_TRISTATE:
> > + switch (sym_get_tristate_value(sym)) {
> > + case no:
> > + fprintf(out, "# CONFIG_%s is not set\n", sym->name);
> > + break;
> > + case yes:
> > + fprintf(out, "CONFIG_%s=y\n", sym->name);
> > + break;
> > + case mod:
> > + fprintf(out, "CONFIG_%s=m\n", sym->name);
> > + break;
> > + }
> > + break;
> > + case S_STRING:
> > + str = sym_get_string_value(sym);
> > + fprintf(out, "CONFIG_%s=\"", sym->name);
> > + while (1) {
> > + l = strcspn(str, "\"\\");
> > + if (l) {
> > + fwrite(str, l, 1, out);
> > + str += l;
> > + }
> > + if (!*str)
> > + break;
> > + fprintf(out, "\\%c", *str++);
> > + }
> > + fputs("\"\n", out);
> > + break;
> > + case S_HEX:
> > + case S_INT:
> > + str = sym_get_string_value(sym);
> > + fprintf(out, "CONFIG_%s=%s\n", sym->name, str);
> > + break;
> > + default:
> > + break;
> > + }
> > +next_symbol:
> > + ;
> > + }
>
> The gotos should be replaced by simple continue statements if there is
> nothing to do after the label.
In an earlier version I jumped to the goto inside a nested for loop.
It is gone so I will use continue as you recommend.
>
> (maybe there are more issues, I didn't look closely into it. But I like
> the approach.)
Likely - I will take a critical look when I respin the patches.
Thanks for the review!
Sam
Hi Sam,
On Sun, Jul 25, 2010 at 11:38:08PM +0200, Sam Ravnborg wrote:
> After reading some of the mails about the
> defconfig files I decided to take a look at kconfig.
>
> We already decided to go for a solution where the defconfig
> files contains only the minimal configuration.
> Uwe developed a script to assist but having native support
> in kconfig is preferable.
>
> This patch set introduce "alldefconfig".
> alldefconfig create a configuration with all values set
> to default values.
> This can be usefull when we check if we do have a sane
> set of default values. Rhe config can be inspected
> using menuconfig.
>
> The patches also introduce "savedefconfig" which
> reads the current configuration and save it
> as a minimal configuration.
>
> In my testing I benchmarked it with one
> of the arm defconfings and the final result is
> the same (symbols appear in different order).
I would prefer to have them in the same order. I havn't checked the
details, but I really like getting support for minimal defconfigs done
in kconfig directly.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Hi Michal.
I cooked up the following to introduce long options in conf.
But in the process I dropped the short options. Is this OK?
Usign the same name in conf.c as used as make target really
helped the readability.
Note: Patch on top of kbuild.get for-next.
It is an RFC patch as I plan to submit a larger serie
when I get the defconfig stuff redone.
But you requested this and it was a logical first step.
Sam
>From 95feb38e5411958df4c5cf7c2e11a9cb53c11c1a Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Wed, 28 Jul 2010 22:25:04 +0200
Subject: [PATCH] kconfig: use long options in conf
The list of options supported by conf is growing
and their abbreviation did not resemble anything usefull.
So drop the single letter options in favour of long options.
The long options are named equal to what we know from
the make target.
The internal implmentation was changed to match this,
resulting in much more readable code.
Support for short options is dropped - no one is supposed
to call this program direct anyway.
Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/kconfig/Makefile | 64 ++++++++----------
scripts/kconfig/conf.c | 165 ++++++++++++++++++++++------------------------
2 files changed, 108 insertions(+), 121 deletions(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 0a34335..ed3f157 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -21,17 +21,17 @@ menuconfig: $(obj)/mconf
$< $(Kconfig)
config: $(obj)/conf
- $< $(Kconfig)
+ $< --oldaskconfig $(Kconfig)
nconfig: $(obj)/nconf
$< $(Kconfig)
oldconfig: $(obj)/conf
- $< -o $(Kconfig)
+ $< --$@ $(Kconfig)
silentoldconfig: $(obj)/conf
$(Q)mkdir -p include/generated
- $< -s $(Kconfig)
+ $< --$@ $(Kconfig)
# if no path is given, then use src directory to find file
ifdef LSMOD
@@ -44,15 +44,15 @@ endif
localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
$(Q)mkdir -p include/generated
$(Q)perl $< $(srctree) $(Kconfig) $(LSMOD_F) > .tmp.config
- $(Q)if [ -f .config ]; then \
- cmp -s .tmp.config .config || \
- (mv -f .config .config.old.1; \
- mv -f .tmp.config .config; \
- $(obj)/conf -s $(Kconfig); \
- mv -f .config.old.1 .config.old) \
- else \
- mv -f .tmp.config .config; \
- $(obj)/conf -s $(Kconfig); \
+ $(Q)if [ -f .config ]; then \
+ cmp -s .tmp.config .config || \
+ (mv -f .config .config.old.1; \
+ mv -f .tmp.config .config; \
+ $(obj)/conf --silentoldconfig $(Kconfig); \
+ mv -f .config.old.1 .config.old) \
+ else \
+ mv -f .tmp.config .config; \
+ $(obj)/conf --silentoldconfig $(Kconfig); \
fi
$(Q)rm -f .tmp.config
@@ -60,23 +60,23 @@ localyesconfig: $(obj)/streamline_config.pl $(obj)/conf
$(Q)mkdir -p include/generated
$(Q)perl $< $(srctree) $(Kconfig) $(LSMOD_F) > .tmp.config
$(Q)sed -i s/=m/=y/ .tmp.config
- $(Q)if [ -f .config ]; then \
- cmp -s .tmp.config .config || \
- (mv -f .config .config.old.1; \
- mv -f .tmp.config .config; \
- $(obj)/conf -s $(Kconfig); \
- mv -f .config.old.1 .config.old) \
- else \
- mv -f .tmp.config .config; \
- $(obj)/conf -s $(Kconfig); \
+ $(Q)if [ -f .config ]; then \
+ cmp -s .tmp.config .config || \
+ (mv -f .config .config.old.1; \
+ mv -f .tmp.config .config; \
+ $(obj)/conf --silentoldconfig $(Kconfig); \
+ mv -f .config.old.1 .config.old) \
+ else \
+ mv -f .tmp.config .config; \
+ $(obj)/conf --silentoldconfig $(Kconfig); \
fi
$(Q)rm -f .tmp.config
nonint_oldconfig: $(obj)/conf
- $< -b $(Kconfig)
+ $< --$@ $(Kconfig)
loose_nonint_oldconfig: $(obj)/conf
- $< -B $(Kconfig)
+ $< --$@ $(Kconfig)
# Create new linux.pot file
# Adjust charset to UTF-8 in .po file to accept UTF-8 in Kconfig files
@@ -104,27 +104,21 @@ update-po-config: $(obj)/kxgettext $(obj)/gconf.glade.h
PHONY += randconfig allyesconfig allnoconfig allmodconfig defconfig
randconfig: $(obj)/conf
- $< -r $(Kconfig)
-
-allyesconfig: $(obj)/conf
- $< -y $(Kconfig)
-
-allnoconfig: $(obj)/conf
- $< -n $(Kconfig)
+ $< --allrandconfig $(Kconfig)
-allmodconfig: $(obj)/conf
- $< -m $(Kconfig)
+allnoconfig allyesconfig allmodconfig: $(obj)/conf
+ $< --$@ $(Kconfig)
defconfig: $(obj)/conf
ifeq ($(KBUILD_DEFCONFIG),)
- $< -d $(Kconfig)
+ $< --defconfig $(Kconfig)
else
@echo "*** Default configuration is based on '$(KBUILD_DEFCONFIG)'"
- $(Q)$< -D arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
+ $(Q)$< --readdefconfig=arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
endif
%_defconfig: $(obj)/conf
- $(Q)$< -D arch/$(SRCARCH)/configs/$@ $(Kconfig)
+ $(Q)$< --readdefconfig=arch/$(SRCARCH)/configs/$@ $(Kconfig)
# Help text used by make help
help:
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index bde01b4..36bd331 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -10,6 +10,7 @@
#include <string.h>
#include <time.h>
#include <unistd.h>
+#include <getopt.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -23,18 +24,20 @@
static void conf(struct menu *menu);
static void check_conf(struct menu *menu);
-enum {
- ask_all,
- ask_new,
- ask_silent,
- dont_ask,
- dont_ask_dont_tell,
- set_default,
- set_yes,
- set_mod,
- set_no,
- set_random
-} input_mode = ask_all;
+enum input_mode {
+ oldaskconfig,
+ silentoldconfig,
+ oldconfig,
+ allnoconfig,
+ allyesconfig,
+ allmodconfig,
+ allrandconfig,
+ defconfig,
+ readdefconfig,
+ nonint_oldconfig,
+ loose_nonint_oldconfig,
+} input_mode = oldaskconfig;
+
char *defconfig_file;
static int indent = 1;
@@ -100,14 +103,14 @@ static int conf_askvalue(struct symbol *sym, const char *def)
}
switch (input_mode) {
- case ask_new:
- case ask_silent:
+ case oldconfig:
+ case silentoldconfig:
if (sym_has_value(sym)) {
printf("%s\n", def);
return 0;
}
check_stdin();
- case ask_all:
+ case oldaskconfig:
fflush(stdout);
fgets(line, 128, stdin);
return 1;
@@ -297,15 +300,15 @@ static int conf_choice(struct menu *menu)
printf("?");
printf("]: ");
switch (input_mode) {
- case ask_new:
- case ask_silent:
+ case oldconfig:
+ case silentoldconfig:
if (!is_new) {
cnt = def;
printf("%d\n", cnt);
break;
}
check_stdin();
- case ask_all:
+ case oldaskconfig:
fflush(stdout);
fgets(line, 128, stdin);
strip(line);
@@ -363,9 +366,9 @@ static void conf(struct menu *menu)
switch (prop->type) {
case P_MENU:
- if ((input_mode == ask_silent ||
- input_mode == dont_ask ||
- input_mode == dont_ask_dont_tell) &&
+ if ((input_mode == silentoldconfig ||
+ input_mode == nonint_oldconfig ||
+ input_mode == loose_nonint_oldconfig) &&
rootEntry != menu) {
check_conf(menu);
return;
@@ -424,9 +427,9 @@ static void check_conf(struct menu *menu)
if (sym && !sym_has_value(sym)) {
if (sym_is_changable(sym) ||
(sym_is_choice(sym) && sym_get_tristate_value(sym) == yes)) {
- if (input_mode == dont_ask ||
- input_mode == dont_ask_dont_tell) {
- if (input_mode == dont_ask &&
+ if (input_mode == nonint_oldconfig ||
+ input_mode == loose_nonint_oldconfig) {
+ if (input_mode == nonint_oldconfig &&
sym->name && !sym_is_choice_value(sym)) {
if (!unset_variables)
fprintf(stderr, "The following"
@@ -448,6 +451,21 @@ static void check_conf(struct menu *menu)
check_conf(child);
}
+static struct option long_opts[] = {
+ {"oldaskconfig", no_argument, NULL, oldaskconfig},
+ {"oldconfig", no_argument, NULL, oldconfig},
+ {"silentoldconfig", no_argument, NULL, silentoldconfig},
+ {"defconfig", no_argument, NULL, defconfig},
+ {"readdefconfig", required_argument, NULL, readdefconfig},
+ {"allnoconfig", no_argument, NULL, allnoconfig},
+ {"allyesconfig", no_argument, NULL, allyesconfig},
+ {"allmodconfig", no_argument, NULL, allmodconfig},
+ {"allrandconfig", no_argument, NULL, allrandconfig},
+ {"nonint_oldconfig", no_argument, NULL, nonint_oldconfig},
+ {"loose_nonint_oldconfig", no_argument, NULL, loose_nonint_oldconfig},
+ {NULL, 0, NULL, 0}
+};
+
int main(int ac, char **av)
{
int opt;
@@ -458,38 +476,16 @@ int main(int ac, char **av)
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((opt = getopt(ac, av, "osbBdD:nmyrh")) != -1) {
+ while ((opt = getopt_long_only(ac, av, "", long_opts, NULL)) != -1) {
+ input_mode = (enum input_mode)opt;
switch (opt) {
- case 'o':
- input_mode = ask_silent;
- break;
- case 's':
- input_mode = ask_silent;
+ case silentoldconfig:
sync_kconfig = 1;
break;
- case 'b':
- input_mode = dont_ask;
- break;
- case 'B':
- input_mode = dont_ask_dont_tell;
- break;
- case 'd':
- input_mode = set_default;
- break;
- case 'D':
- input_mode = set_default;
+ case readdefconfig:
defconfig_file = optarg;
break;
- case 'n':
- input_mode = set_no;
- break;
- case 'm':
- input_mode = set_mod;
- break;
- case 'y':
- input_mode = set_yes;
- break;
- case 'r':
+ case allrandconfig:
{
struct timeval now;
unsigned int seed;
@@ -502,17 +498,12 @@ int main(int ac, char **av)
seed = (unsigned int)((now.tv_sec + 1) * (now.tv_usec + 1));
srand(seed);
-
- input_mode = set_random;
break;
}
- case 'h':
- printf(_("See README for usage info\n"));
- exit(0);
- break;
- default:
+ case '?':
fprintf(stderr, _("See README for usage info\n"));
exit(1);
+ break;
}
}
if (ac == optind) {
@@ -537,7 +528,8 @@ int main(int ac, char **av)
}
switch (input_mode) {
- case set_default:
+ case defconfig:
+ case readdefconfig:
if (!defconfig_file)
defconfig_file = conf_get_default_confname();
if (conf_read(defconfig_file)) {
@@ -547,27 +539,27 @@ int main(int ac, char **av)
exit(1);
}
break;
- case ask_silent:
- case ask_all:
- case ask_new:
- case dont_ask:
- case dont_ask_dont_tell:
+ case silentoldconfig:
+ case oldaskconfig:
+ case oldconfig:
+ case nonint_oldconfig:
+ case loose_nonint_oldconfig:
conf_read(NULL);
break;
- case set_no:
- case set_mod:
- case set_yes:
- case set_random:
+ case allnoconfig:
+ case allyesconfig:
+ case allmodconfig:
+ case allrandconfig:
name = getenv("KCONFIG_ALLCONFIG");
if (name && !stat(name, &tmpstat)) {
conf_read_simple(name, S_DEF_USER);
break;
}
switch (input_mode) {
- case set_no: name = "allno.config"; break;
- case set_mod: name = "allmod.config"; break;
- case set_yes: name = "allyes.config"; break;
- case set_random: name = "allrandom.config"; break;
+ case allnoconfig: name = "allno.config"; break;
+ case allyesconfig: name = "allyes.config"; break;
+ case allmodconfig: name = "allmod.config"; break;
+ case allrandconfig: name = "allrandom.config"; break;
default: break;
}
if (!stat(name, &tmpstat))
@@ -592,37 +584,38 @@ int main(int ac, char **av)
}
switch (input_mode) {
- case set_no:
+ case allnoconfig:
conf_set_all_new_symbols(def_no);
break;
- case set_yes:
+ case allyesconfig:
conf_set_all_new_symbols(def_yes);
break;
- case set_mod:
+ case allmodconfig:
conf_set_all_new_symbols(def_mod);
break;
- case set_random:
+ case allrandconfig:
conf_set_all_new_symbols(def_random);
break;
- case set_default:
+ case defconfig:
+ case readdefconfig:
conf_set_all_new_symbols(def_default);
break;
- case ask_new:
- case ask_all:
+ case oldconfig:
+ case oldaskconfig:
rootEntry = &rootmenu;
conf(&rootmenu);
- input_mode = ask_silent;
+ input_mode = silentoldconfig;
/* fall through */
- case dont_ask:
- case dont_ask_dont_tell:
- case ask_silent:
+ case nonint_oldconfig:
+ case loose_nonint_oldconfig:
+ case silentoldconfig:
/* Update until a loop caused no more changes */
do {
conf_cnt = 0;
check_conf(&rootmenu);
} while (conf_cnt &&
- (input_mode != dont_ask &&
- input_mode != dont_ask_dont_tell));
+ (input_mode != nonint_oldconfig &&
+ input_mode != loose_nonint_oldconfig));
break;
}
@@ -638,7 +631,7 @@ int main(int ac, char **av)
fprintf(stderr, _("\n*** Error during update of the kernel configuration.\n\n"));
return 1;
}
- } else if (!unset_variables || input_mode != dont_ask) {
+ } else if (!unset_variables || input_mode != nonint_oldconfig) {
if (conf_write(NULL)) {
fprintf(stderr, _("\n*** Error during writing of the kernel configuration.\n\n"));
exit(1);
--
1.6.0.6
On Wed, Jul 28, 2010 at 10:36:22PM +0200, Sam Ravnborg wrote:
> Hi Michal.
>
> I cooked up the following to introduce long options in conf.
> But in the process I dropped the short options. Is this OK?
On top of this patch I did another two.
They fixup the *nonint_oldconfig targets:
nonint_oldconfig:
- renamed to listnewconfig
- print new options to stdout (to better support redirect)
- no longer saves a new configuration
- does ot exist with a failure code if there is new options
loose_noninit_oldconfig:
- renamed to oldnoconfig
- does ot exist with a failure code if there is new options
This is a lot cleaner and much more sensible names.
And they still support the suecase where they
list new options and can set them to n per default.
[cc: list trimmed on patches]
Sam
Sam Ravnborg (2):
kconfig: rename loose_nonint_oldconfig => oldnoconfig
kconfig: change nonint_oldconfig to listnewconfig
scripts/kconfig/Makefile | 12 +++++-------
scripts/kconfig/conf.c | 46 +++++++++++++++++-----------------------------
2 files changed, 22 insertions(+), 36 deletions(-)
>From a76c17f4fa213ad68227456374be6c84c825c3a5 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Thu, 29 Jul 2010 09:34:43 +0200
Subject: [PATCH 1/2] kconfig: rename loose_nonint_oldconfig => oldnoconfig
Rename target to something that fall more in line
with the other kconfig targets.
oldnoconfig shall read as:
- read the old configuration and set all new options to no
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: Aristeu Rozanski <[email protected]>
---
scripts/kconfig/Makefile | 9 ++++-----
scripts/kconfig/conf.c | 14 +++++++-------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index ed3f157..6c71306 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,7 +3,7 @@
# These targets are used from top-level makefile
PHONY += oldconfig xconfig gconfig menuconfig config silentoldconfig update-po-config \
- localmodconfig localyesconfig
+ localmodconfig localyesconfig oldnoconfig
ifdef KBUILD_KCONFIG
Kconfig := $(KBUILD_KCONFIG)
@@ -72,10 +72,10 @@ localyesconfig: $(obj)/streamline_config.pl $(obj)/conf
fi
$(Q)rm -f .tmp.config
-nonint_oldconfig: $(obj)/conf
+nonit_oldconfig: $(obj)/conf
$< --$@ $(Kconfig)
-loose_nonint_oldconfig: $(obj)/conf
+oldnoconfig: $(obj)/conf
$< --$@ $(Kconfig)
# Create new linux.pot file
@@ -138,8 +138,7 @@ help:
@echo ' allnoconfig - New config where all options are answered with no'
@echo ' nonint_oldconfig - Checks the current configuration and fails if an option is '
@echo ' not set'
- @echo ' loose_nonint_oldconfig - Same as nonint_oldconfig, but updates the config file with '
- @echo ' missing config options as unset'
+ @echo ' oldnoconfig - Same as silentoldconfig but set new symbols to n (unset)'
# lxdialog stuff
check-lxdialog := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 36bd331..3c8f31b 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -35,7 +35,7 @@ enum input_mode {
defconfig,
readdefconfig,
nonint_oldconfig,
- loose_nonint_oldconfig,
+ oldnoconfig,
} input_mode = oldaskconfig;
char *defconfig_file;
@@ -368,7 +368,7 @@ static void conf(struct menu *menu)
case P_MENU:
if ((input_mode == silentoldconfig ||
input_mode == nonint_oldconfig ||
- input_mode == loose_nonint_oldconfig) &&
+ input_mode == oldnoconfig) &&
rootEntry != menu) {
check_conf(menu);
return;
@@ -428,7 +428,7 @@ static void check_conf(struct menu *menu)
if (sym_is_changable(sym) ||
(sym_is_choice(sym) && sym_get_tristate_value(sym) == yes)) {
if (input_mode == nonint_oldconfig ||
- input_mode == loose_nonint_oldconfig) {
+ input_mode == oldnoconfig) {
if (input_mode == nonint_oldconfig &&
sym->name && !sym_is_choice_value(sym)) {
if (!unset_variables)
@@ -462,7 +462,7 @@ static struct option long_opts[] = {
{"allmodconfig", no_argument, NULL, allmodconfig},
{"allrandconfig", no_argument, NULL, allrandconfig},
{"nonint_oldconfig", no_argument, NULL, nonint_oldconfig},
- {"loose_nonint_oldconfig", no_argument, NULL, loose_nonint_oldconfig},
+ {"oldnoconfig", no_argument, NULL, oldnoconfig},
{NULL, 0, NULL, 0}
};
@@ -543,7 +543,7 @@ int main(int ac, char **av)
case oldaskconfig:
case oldconfig:
case nonint_oldconfig:
- case loose_nonint_oldconfig:
+ case oldnoconfig:
conf_read(NULL);
break;
case allnoconfig:
@@ -607,7 +607,7 @@ int main(int ac, char **av)
input_mode = silentoldconfig;
/* fall through */
case nonint_oldconfig:
- case loose_nonint_oldconfig:
+ case oldnoconfig:
case silentoldconfig:
/* Update until a loop caused no more changes */
do {
@@ -615,7 +615,7 @@ int main(int ac, char **av)
check_conf(&rootmenu);
} while (conf_cnt &&
(input_mode != nonint_oldconfig &&
- input_mode != loose_nonint_oldconfig));
+ input_mode != oldnoconfig));
break;
}
--
1.6.0.6
>From 7b28159d4367889157f6371cdd3dd34138be3699 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Thu, 29 Jul 2010 09:47:26 +0200
Subject: [PATCH 2/2] kconfig: change nonint_oldconfig to listnewconfig
Rename to a name that better match the other kconfig targets.
listnewconfig shall read as:
- list all new options compared to current configuration
Also change so we do one thing - which is to list new options.
The configuration is not updated.
New options are now written to stdout so one can redirect the output.
Finally drop the failure code. We list new options if there are any.
The failure code is dropped from both oldnoconfig and listnewconfig.
These are all feature changes compared to the original
nonint_oldconfig - but as this feature has not yet been in a
released kernel it should not matter.
It is still possible to do:
make listnewconfig
lookup new config options in Kconfig*
edit .config
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: Aristeu Rozanski <[email protected]>
---
scripts/kconfig/Makefile | 7 +++----
scripts/kconfig/conf.c | 34 +++++++++++-----------------------
2 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 6c71306..1290ac3 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,7 +3,7 @@
# These targets are used from top-level makefile
PHONY += oldconfig xconfig gconfig menuconfig config silentoldconfig update-po-config \
- localmodconfig localyesconfig oldnoconfig
+ localmodconfig localyesconfig oldnoconfig listnewconfig
ifdef KBUILD_KCONFIG
Kconfig := $(KBUILD_KCONFIG)
@@ -72,7 +72,7 @@ localyesconfig: $(obj)/streamline_config.pl $(obj)/conf
fi
$(Q)rm -f .tmp.config
-nonit_oldconfig: $(obj)/conf
+listnewconfig: $(obj)/conf
$< --$@ $(Kconfig)
oldnoconfig: $(obj)/conf
@@ -136,8 +136,7 @@ help:
@echo ' allmodconfig - New config selecting modules when possible'
@echo ' allyesconfig - New config where all options are accepted with yes'
@echo ' allnoconfig - New config where all options are answered with no'
- @echo ' nonint_oldconfig - Checks the current configuration and fails if an option is '
- @echo ' not set'
+ @echo ' listnewconfig - List new options'
@echo ' oldnoconfig - Same as silentoldconfig but set new symbols to n (unset)'
# lxdialog stuff
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 3c8f31b..b917778 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -17,10 +17,6 @@
#define LKC_DIRECT_LINK
#include "lkc.h"
-/* Return codes */
-#define EUNSETOPT 2 /* if -B and -b are used and unset config
- * options were found */
-
static void conf(struct menu *menu);
static void check_conf(struct menu *menu);
@@ -34,7 +30,7 @@ enum input_mode {
allrandconfig,
defconfig,
readdefconfig,
- nonint_oldconfig,
+ listnewconfig,
oldnoconfig,
} input_mode = oldaskconfig;
@@ -46,7 +42,6 @@ static int sync_kconfig;
static int conf_cnt;
static char line[128];
static struct menu *rootEntry;
-static int unset_variables;
static void print_help(struct menu *menu)
{
@@ -367,7 +362,7 @@ static void conf(struct menu *menu)
switch (prop->type) {
case P_MENU:
if ((input_mode == silentoldconfig ||
- input_mode == nonint_oldconfig ||
+ input_mode == listnewconfig ||
input_mode == oldnoconfig) &&
rootEntry != menu) {
check_conf(menu);
@@ -427,16 +422,9 @@ static void check_conf(struct menu *menu)
if (sym && !sym_has_value(sym)) {
if (sym_is_changable(sym) ||
(sym_is_choice(sym) && sym_get_tristate_value(sym) == yes)) {
- if (input_mode == nonint_oldconfig ||
- input_mode == oldnoconfig) {
- if (input_mode == nonint_oldconfig &&
- sym->name && !sym_is_choice_value(sym)) {
- if (!unset_variables)
- fprintf(stderr, "The following"
- " variables are not set:\n");
- fprintf(stderr, "CONFIG_%s\n",
- sym->name);
- unset_variables++;
+ if (input_mode == listnewconfig) {
+ if (sym->name && !sym_is_choice_value(sym)) {
+ printf("CONFIG_%s\n", sym->name);
}
} else {
if (!conf_cnt++)
@@ -461,7 +449,7 @@ static struct option long_opts[] = {
{"allyesconfig", no_argument, NULL, allyesconfig},
{"allmodconfig", no_argument, NULL, allmodconfig},
{"allrandconfig", no_argument, NULL, allrandconfig},
- {"nonint_oldconfig", no_argument, NULL, nonint_oldconfig},
+ {"listnewconfig", no_argument, NULL, listnewconfig},
{"oldnoconfig", no_argument, NULL, oldnoconfig},
{NULL, 0, NULL, 0}
};
@@ -542,7 +530,7 @@ int main(int ac, char **av)
case silentoldconfig:
case oldaskconfig:
case oldconfig:
- case nonint_oldconfig:
+ case listnewconfig:
case oldnoconfig:
conf_read(NULL);
break;
@@ -606,7 +594,7 @@ int main(int ac, char **av)
conf(&rootmenu);
input_mode = silentoldconfig;
/* fall through */
- case nonint_oldconfig:
+ case listnewconfig:
case oldnoconfig:
case silentoldconfig:
/* Update until a loop caused no more changes */
@@ -614,7 +602,7 @@ int main(int ac, char **av)
conf_cnt = 0;
check_conf(&rootmenu);
} while (conf_cnt &&
- (input_mode != nonint_oldconfig &&
+ (input_mode != listnewconfig &&
input_mode != oldnoconfig));
break;
}
@@ -631,11 +619,11 @@ int main(int ac, char **av)
fprintf(stderr, _("\n*** Error during update of the kernel configuration.\n\n"));
return 1;
}
- } else if (!unset_variables || input_mode != nonint_oldconfig) {
+ } else if (input_mode != listnewconfig) {
if (conf_write(NULL)) {
fprintf(stderr, _("\n*** Error during writing of the kernel configuration.\n\n"));
exit(1);
}
}
- return unset_variables ? EUNSETOPT : 0;
+ return 0;
}
--
1.6.0.6
On 28.7.2010 22:36, Sam Ravnborg wrote:
> Hi Michal.
>
> I cooked up the following to introduce long options in conf.
> But in the process I dropped the short options. Is this OK?
Nice! I would probably name the option and the enum value for randconfig
simply --randconfig to not introduce new names (the scripts/kconfig/conf
call is displayed to the user).
> Usign the same name in conf.c as used as make target really
> helped the readability.
Indeed.
Michal
On 29.7.2010 10:13, Sam Ravnborg wrote:
> On Wed, Jul 28, 2010 at 10:36:22PM +0200, Sam Ravnborg wrote:
>> Hi Michal.
>>
>> I cooked up the following to introduce long options in conf.
>> But in the process I dropped the short options. Is this OK?
>
> On top of this patch I did another two.
> They fixup the *nonint_oldconfig targets:
>
> nonint_oldconfig:
> - renamed to listnewconfig
> - print new options to stdout (to better support redirect)
> - no longer saves a new configuration
> - does ot exist with a failure code if there is new options
>
> loose_noninit_oldconfig:
> - renamed to oldnoconfig
> - does ot exist with a failure code if there is new options
>
> This is a lot cleaner and much more sensible names.
> And they still support the suecase where they
> list new options and can set them to n per default.
I do not object, these new targets are only in the kbuild tree atm, so
users are not relying on them yet. And the new names indeed describe
better what they do. Aristeu?
Michal
On Thu, Jul 29, 2010 at 11:17:46AM +0200, Michal Marek wrote:
> On 28.7.2010 22:36, Sam Ravnborg wrote:
> > Hi Michal.
> >
> > I cooked up the following to introduce long options in conf.
> > But in the process I dropped the short options. Is this OK?
>
> Nice! I would probably name the option and the enum value for randconfig
> simply --randconfig to not introduce new names (the scripts/kconfig/conf
> call is displayed to the user).
I had in mind to deprecate randconfig in favour of allrandconfig
to get more consitency in the names.
But I failed to document this in the changelog.
I can submit the following as a real patch if you agree.
Sam
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 1290ac3..9670323 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -101,12 +101,12 @@ update-po-config: $(obj)/kxgettext $(obj)/gconf.glade.h
$(Q)rm -f arch/um/Kconfig.arch
$(Q)rm -f $(obj)/config.pot
-PHONY += randconfig allyesconfig allnoconfig allmodconfig defconfig
+PHONY += allnoconfig allyesconfig allmodconfig allrandconfig randconfig defconfig
-randconfig: $(obj)/conf
- $< --allrandconfig $(Kconfig)
+# Support deprecated randconfig target
+randconfig: allrandconfig
-allnoconfig allyesconfig allmodconfig: $(obj)/conf
+allnoconfig allyesconfig allmodconfig allrandconfig: $(obj)/conf
$< --$@ $(Kconfig)
defconfig: $(obj)/conf
@@ -131,7 +131,7 @@ help:
@echo ' localmodconfig - Update current config disabling modules not loaded'
@echo ' localyesconfig - Update current config converting local mods to core'
@echo ' silentoldconfig - Same as oldconfig, but quietly, additionally update deps'
- @echo ' randconfig - New config with random answer to all options'
+ @echo ' allrandconfig - New config with random answer to all options'
@echo ' defconfig - New config with default answer to all options'
@echo ' allmodconfig - New config selecting modules when possible'
@echo ' allyesconfig - New config where all options are accepted with yes'
On 29.7.2010 11:39, Sam Ravnborg wrote:
> I had in mind to deprecate randconfig in favour of allrandconfig
> to get more consitency in the names.
> But I failed to document this in the changelog.
>
> I can submit the following as a real patch if you agree.
To me it seems like a change for the sake of change :), but if you like
to have the targets named all*config consistenly, I'm OK with it.
Michal
On Thu, 29 Jul 2010 12:08:24 +0200 Michal Marek wrote:
> On 29.7.2010 11:39, Sam Ravnborg wrote:
> > I had in mind to deprecate randconfig in favour of allrandconfig
> > to get more consitency in the names.
> > But I failed to document this in the changelog.
> >
> > I can submit the following as a real patch if you agree.
>
> To me it seems like a change for the sake of change :), but if you like
> to have the targets named all*config consistenly, I'm OK with it.
Well, there are scripts that know the old name, so if it's just change
for the sake of change, let's not.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On Thu, Jul 29, 2010 at 11:23:49AM +0200, Michal Marek wrote:
> On 29.7.2010 10:13, Sam Ravnborg wrote:
> > On Wed, Jul 28, 2010 at 10:36:22PM +0200, Sam Ravnborg wrote:
> >> Hi Michal.
> >>
> >> I cooked up the following to introduce long options in conf.
> >> But in the process I dropped the short options. Is this OK?
> >
> > On top of this patch I did another two.
> > They fixup the *nonint_oldconfig targets:
> >
> > nonint_oldconfig:
> > - renamed to listnewconfig
> > - print new options to stdout (to better support redirect)
> > - no longer saves a new configuration
> > - does ot exist with a failure code if there is new options
but this kills its usefulness. nonint_oldconfig is used so you can script
the use of a generated configuration (think distro kernel RPMs). if something
is not set, it fails listing what's not set. otherwise it'll save the
configuration and whatever script is using it can proceed.
"listnewconfig" is a new, different target to me.
> > loose_noninit_oldconfig:
> > - renamed to oldnoconfig
> > - does ot exist with a failure code if there is new options
> >
> > This is a lot cleaner and much more sensible names.
> > And they still support the suecase where they
> > list new options and can set them to n per default.
>
> I do not object, these new targets are only in the kbuild tree atm, so
> users are not relying on them yet. And the new names indeed describe
> better what they do. Aristeu?
oldnoconfig is fine to me.
--
Aristeu
On 29.7.2010 16:47, Aristeu Rozanski wrote:
> On Thu, Jul 29, 2010 at 11:23:49AM +0200, Michal Marek wrote:
>> On 29.7.2010 10:13, Sam Ravnborg wrote:
>>> On Wed, Jul 28, 2010 at 10:36:22PM +0200, Sam Ravnborg wrote:
>>>> Hi Michal.
>>>>
>>>> I cooked up the following to introduce long options in conf.
>>>> But in the process I dropped the short options. Is this OK?
>>>
>>> On top of this patch I did another two.
>>> They fixup the *nonint_oldconfig targets:
>>>
>>> nonint_oldconfig:
>>> - renamed to listnewconfig
>>> - print new options to stdout (to better support redirect)
>>> - no longer saves a new configuration
>>> - does ot exist with a failure code if there is new options
> but this kills its usefulness. nonint_oldconfig is used so you can script
> the use of a generated configuration (think distro kernel RPMs). if something
> is not set, it fails listing what's not set. otherwise it'll save the
> configuration and whatever script is using it can proceed.
> "listnewconfig" is a new, different target to me.
How about
new=$(make listnewconfig)
if test -n "$new"; then
echo "Please set the following options:" >&2
echo "$new" >&2
exit 1
fi
? Wouldn't that be the same as nonint_oldconfig before?
Michal
On Thu, Jul 29, 2010 at 05:04:57PM +0200, Michal Marek wrote:
> On 29.7.2010 16:47, Aristeu Rozanski wrote:
> > On Thu, Jul 29, 2010 at 11:23:49AM +0200, Michal Marek wrote:
> >> On 29.7.2010 10:13, Sam Ravnborg wrote:
> >>> On Wed, Jul 28, 2010 at 10:36:22PM +0200, Sam Ravnborg wrote:
> >>>> Hi Michal.
> >>>>
> >>>> I cooked up the following to introduce long options in conf.
> >>>> But in the process I dropped the short options. Is this OK?
> >>>
> >>> On top of this patch I did another two.
> >>> They fixup the *nonint_oldconfig targets:
> >>>
> >>> nonint_oldconfig:
> >>> - renamed to listnewconfig
> >>> - print new options to stdout (to better support redirect)
> >>> - no longer saves a new configuration
> >>> - does ot exist with a failure code if there is new options
> > but this kills its usefulness. nonint_oldconfig is used so you can script
> > the use of a generated configuration (think distro kernel RPMs). if something
> > is not set, it fails listing what's not set. otherwise it'll save the
> > configuration and whatever script is using it can proceed.
> > "listnewconfig" is a new, different target to me.
>
> How about
> new=$(make listnewconfig)
> if test -n "$new"; then
> echo "Please set the following options:" >&2
> echo "$new" >&2
> exit 1
> fi
> ? Wouldn't that be the same as nonint_oldconfig before?
what's the other use cases for listnewconfig (other than a incomplete
nonint_oldconfig)?
--
Aristeu
On Thu, Jul 29, 2010 at 07:30:16AM -0700, Randy Dunlap wrote:
> On Thu, 29 Jul 2010 12:08:24 +0200 Michal Marek wrote:
>
> > On 29.7.2010 11:39, Sam Ravnborg wrote:
> > > I had in mind to deprecate randconfig in favour of allrandconfig
> > > to get more consitency in the names.
> > > But I failed to document this in the changelog.
> > >
> > > I can submit the following as a real patch if you agree.
> >
> > To me it seems like a change for the sake of change :), but if you like
> > to have the targets named all*config consistenly, I'm OK with it.
>
>
> Well, there are scripts that know the old name, so if it's just change
> for the sake of change, let's not.
I will respin the patches where I drop this change and keep "randconfig".
Thanks for input from both of you!
Sam
Hi Aristeu.
> > >>> nonint_oldconfig:
> > >>> - renamed to listnewconfig
> > >>> - print new options to stdout (to better support redirect)
> > >>> - no longer saves a new configuration
> > >>> - does ot exist with a failure code if there is new options
> > > but this kills its usefulness. nonint_oldconfig is used so you can script
> > > the use of a generated configuration (think distro kernel RPMs). if something
> > > is not set, it fails listing what's not set. otherwise it'll save the
> > > configuration and whatever script is using it can proceed.
> > > "listnewconfig" is a new, different target to me.
If you have a simple command that give you a list of new
symbols then this is easy to script as Michal also
shows with the below example.
> > How about
> > new=$(make listnewconfig)
> > if test -n "$new"; then
> > echo "Please set the following options:" >&2
> > echo "$new" >&2
> > exit 1
> > fi
> > ? Wouldn't that be the same as nonint_oldconfig before?
> what's the other use cases for listnewconfig (other than a incomplete
> nonint_oldconfig)?
listnewconfig is for everyone that like to see a list of new
config options - without touching the current configuration.
By limiting listnewconfig to do only one thing you actually
create further uses than before.
This is not about how well it applies to the tailored
use in redhat's current scripts.
This is about creating a command which by doing only
a single thing (and to do so well) allows others
to utilize this.
Sam
On Thu, Jul 29, 2010 at 09:34:55PM +0200, Sam Ravnborg wrote:
> If you have a simple command that give you a list of new
> symbols then this is easy to script as Michal also
> shows with the below example.
>
> > > How about
> > > new=$(make listnewconfig)
> > > if test -n "$new"; then
> > > echo "Please set the following options:" >&2
> > > echo "$new" >&2
> > > exit 1
> > > fi
> > > ? Wouldn't that be the same as nonint_oldconfig before?
> > what's the other use cases for listnewconfig (other than a incomplete
> > nonint_oldconfig)?
>
> listnewconfig is for everyone that like to see a list of new
> config options - without touching the current configuration.
>
> By limiting listnewconfig to do only one thing you actually
> create further uses than before.
>
> This is not about how well it applies to the tailored
> use in redhat's current scripts.
*sigh* I think we have people able to handle such complex changes.
this is not what it's about. I don't care how it's called or if scripts
will need to be changed. What I want to know is if either:
a) we're reducing functionality of something in order to support more *real*
use cases with the same code, making it more generic;
or
b) we're reducing functionality based in theorical use cases.
if it's (a), you get my ACK
--
Aristeu
On Thu, Jul 29, 2010 at 03:50:52PM -0400, Aristeu Rozanski wrote:
> On Thu, Jul 29, 2010 at 09:34:55PM +0200, Sam Ravnborg wrote:
> > If you have a simple command that give you a list of new
> > symbols then this is easy to script as Michal also
> > shows with the below example.
> >
> > > > How about
> > > > new=$(make listnewconfig)
> > > > if test -n "$new"; then
> > > > echo "Please set the following options:" >&2
> > > > echo "$new" >&2
> > > > exit 1
> > > > fi
> > > > ? Wouldn't that be the same as nonint_oldconfig before?
> > > what's the other use cases for listnewconfig (other than a incomplete
> > > nonint_oldconfig)?
> >
> > listnewconfig is for everyone that like to see a list of new
> > config options - without touching the current configuration.
> >
> > By limiting listnewconfig to do only one thing you actually
> > create further uses than before.
First off let me correct myself.
nonint_oldconfig does the following:
- print any new options to stderr
- if tere are any new options exit with exit code 2
- it does _not_ touch the current configuration (as I wrongly implied above)
listnewconfig does the following:
- print any new options to stdout
- return with exit code 0 (success)
- it does not touch the current configuration
So the change we discuss is only the exit code.
You indicated you were OK with the name change,
and I assume the change from stderr => stdout is OK too.
> >
> > This is not about how well it applies to the tailored
> > use in redhat's current scripts.
> *sigh* I think we have people able to handle such complex changes.
>
> this is not what it's about. I don't care how it's called or if scripts
> will need to be changed. What I want to know is if either:
> a) we're reducing functionality of something in order to support more *real*
> use cases with the same code, making it more generic;
> or
> b) we're reducing functionality based in theorical use cases.
>
> if it's (a), you get my ACK
I can try to come up with some use cases that I consider real...
a) List the number of new options after I upgraded my kernel.
It can be used to judge the amount of time used to answer
oldconfig manually.
If make tells me that conf had an exit code != success how
do I then know the list is complete?
b) List the number of new options when I bring in an old config.
Same comment as in a) about the exit code.
c) List the options I need to carefully lookup to judge what to do about them.
A variant of a) and b) - the one I expect all distributions uses in some way.
[scripts/diffconfig is one way]
d) We can use listnewconfig in scripts without failing due to
conf exit with an error code when there are new options.
And use of stdout is thus more convinient.
[And I know the counter argument is that with the exit code it is easy
to judge if there is any new options].
I hope this shows enough examples that you consider in the _real_
category that you can give it your ack.
Sam
On Fri, Jul 30, 2010 at 01:04:19AM +0200, Sam Ravnborg wrote:
> First off let me correct myself.
> nonint_oldconfig does the following:
> - print any new options to stderr
> - if tere are any new options exit with exit code 2
> - it does _not_ touch the current configuration (as I wrongly implied above)
it does touch the current configuration if there aren't new options
but whatever
Acked-by: Aristeu Rozanski <[email protected]>
--
Aristeu