| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100727165045.GA26649@merkur.ravnborg.org>
Date: Tue, 27 Jul 2010 18:50:45 +0200
From: Sam Ravnborg <sam@...nborg.org>
To: Michal Marek <mmarek@...e.cz>
Cc: lkml <linux-kernel@...r.kernel.org>,
linux-kbuild <linux-kbuild@...r.kernel.org>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Roman Zippel <zippel@...ux-m68k.org>,
Uwe Kleine-Koig <u.kleine-koenig@...gutronix.de>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 4/4] kconfig: add 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.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists