lkml.org 
[lkml]   [2011]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] Enable 'make CONFIG_FOO=y oldconfig'
    From
    Date
    On Fri, 2011-07-29 at 21:15 -0400, Arnaud Lacombe wrote:
    > Technically, kconfig already provides you all the interfaces to set
    > symbol to a given value, for `randconfig' via "rand.config", for
    > `all{yes,mod,no}config', via all.{yes,mod,no}config, for oldconfig via
    > .config. Why another interface ?

    It's useful for some people to be able to do this as simply as possible
    from the command line, and they get very upset if you suggest that they
    might use those other methods instead.

    In particular, some people currently use the legacy 'ARCH=i386' and
    'ARCH=x86_64' settings on the command line in order to switch the value
    of CONFIG_64BIT on and off for various *config targets¹. In fact, I
    think that's the *only* thing that those legacy ARCH settings still do;
    they could (technically, at least) be killed off if we have an
    alternative way of doing it. This provides an alternative, more generic
    way of doing it for *all* config options; not just CONFIG_64BIT and not
    just for x86.

    (¹ Note: in fact, *many* people use 'ARCH=i386' and 'ARCH=x86_64', but
    not really for this purpose. Most people use it just to work around the
    bug I fixed in my other patch — that the value of CONFIG_64BIT is
    *always* overridden to match the build host, unless you do *something*
    on the command line to work around it. Just ARCH=x86 would suffice,
    since that makes CONFIG_64BIT work sanely again.)

    > > +void conf_set_symbol_from_env(char *str)
    > > +{
    > > + char *p = strchr(str, '=');
    > > + struct symbol *sym;
    > > + int def = S_DEF_USER;
    > > + int def_flags = SYMBOL_DEF << def;
    > > +
    > > + if (!p)
    > > + return;
    > the environment should already gives you the guarantee that `p' won't
    > be NULL. If that's not the case, the environment is likely to be
    > broken.

    Yes, that's true. Nevertheless, I've seen broken environments — there's
    no harm in being robust. We can't be robust in the face of *all* the
    ways the environment could screw up, of course, but this check doesn't
    really hurt. Especially given that the getenv() isn't even *in* this
    function, so this function is even more justified in checking its
    inputs.

    > > +
    > > + *p = 0;
    > > + sym = sym_find(str + strlen(CONFIG_));
    > > + *p++ = '=';
    > > +
    > > + if (!sym)
    > > + return;
    > > +
    > > + sym_calc_value(sym);
    > > + if (!sym_set_string_value(sym, p))
    > > + return;
    > > + conf_message("CONFIG_%s set to %s from environment", sym->name, p);
    > please do not hardcode the prefix.

    OK.

    > > + if (sym && sym_is_choice_value(sym)) {
    > you do not need to test for the validity of a pointer you
    > unconditionally dereference right before :)

    That's a copy-and-paste from identical code in conf_read_simple(), which
    I suppose should be factored out into a separate function. if I really
    understood what it was doing I'd be able to suggest a sane name...
    sym_validate_choice_state() perhaps?

    I'll do that.

    > > + struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
    > > + switch (sym->def[def].tri) {
    > > + case no:
    > > + break;
    > > + case mod:
    > > + if (cs->def[def].tri == yes) {
    > > + conf_warning("%s creates inconsistent choice state", sym->name);
    > > + cs->flags &= ~def_flags;
    > > + }
    > > + break;
    > > + case yes:
    > > + if (cs->def[def].tri != no)
    > > + conf_warning("override: %s changes choice state", sym->name);
    > > + cs->def[def].val = sym;
    > > + break;
    > > + }
    > > + cs->def[def].tri = EXPR_OR(cs->def[def].tri, sym->def[def].tri);
    > > + }
    > > +}
    > > +
    --
    dwmw2

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-07-30 11:07    [W:5.508 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site