| Message ID | giob664yy1.fsf@karga.hank.lab (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Dirk Gouders | 2013-10-30 15:26:30 [+0100]: >Hi Sebastian, Yann, all, Hi Dirk, >below is a patch that prevents choice_values to appear in the list if >they depend on 'm' symbols and the choice symbol is set to 'y'. I would >be glad if you could have a look at it. I applied this patch (and ignored the other one in this thread) and the problem Tomi Valkeinen noticed and I reported here seems to be gone. Thank you very much. >Dirk Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dirk, All, On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly: > Dirk Gouders <dirk@gouders.net> writes: [--SNIP--] > below is a patch that prevents choice_values to appear in the list if > they depend on 'm' symbols and the choice symbol is set to 'y'. I would > be glad if you could have a look at it. Next time, could you use 'git send-email' to send your patches, it makes reviewing and commenting much simpler. Also, it does not mangle the patch commit, and thus makes it easier to apply with 'git am'. > Yann, in this patch I wrote " if (choice_sym != NULL..." -- I guess that > is one point that you will probably remark in the previous patch. When we check that a pointer is valid, there's no reason to check it against NULL, just: if (choice_sym && choice_sym->...) > Another point is that all the conditionals in both patches could be I am not sure I understand what issue this patch(es) are supposed to fix. What bothers me is that they touch two different places in the code, yet have the same subject, so it seems both are tryiong to fix the same issue. Do you intend that both patches should be applied, or that this second one supersedes the previous one? > limited to only those choice_values that are of type tristate but I > think that makes the code even harder to read... Yes, and it does not matter since non-trisates are necessarily booleans, and those are covered since they will never be '== mod', so their visibility was, and still is, properly handled. > From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001 > From: Dirk Gouders <dirk@gouders.net> > Date: Wed, 30 Oct 2013 15:06:05 +0100 > Subject: [PATCH 2/2] kconfig/symbol.c: handle visibility of choice_values that > depend on 'm' symbols > > If choice_values depend on symbols that are set to 'm' then these > choice_values should not be visible in choice lists if the choice > symbol is set to 'y'. See USB Gadget Drivers, for example. I liked your previous commit message better, because it had a test-case that did exhibit the problem. Can you please: - confirm which patch/es is/are needed - update the commit log/s back with the test-case/s - resend needed patch/es Thank you! Regards, Yann E. MORIN.
"Yann E. MORIN" <yann.morin.1998@free.fr> writes: > Dirk, All, > > On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly: >> Dirk Gouders <dirk@gouders.net> writes: > [--SNIP--] >> below is a patch that prevents choice_values to appear in the list if >> they depend on 'm' symbols and the choice symbol is set to 'y'. I would >> be glad if you could have a look at it. > > Next time, could you use 'git send-email' to send your patches, it makes > reviewing and commenting much simpler. > > Also, it does not mangle the patch commit, and thus makes it easier to > apply with 'git am'. > >> Yann, in this patch I wrote " if (choice_sym != NULL..." -- I guess that >> is one point that you will probably remark in the previous patch. > > When we check that a pointer is valid, there's no reason to check it > against NULL, just: > if (choice_sym && choice_sym->...) > >> Another point is that all the conditionals in both patches could be > > I am not sure I understand what issue this patch(es) are supposed to > fix. > > What bothers me is that they touch two different places in the code, yet > have the same subject, so it seems both are tryiong to fix the same > issue. > > Do you intend that both patches should be applied, or that this second > one supersedes the previous one? > >> limited to only those choice_values that are of type tristate but I >> think that makes the code even harder to read... > > Yes, and it does not matter since non-trisates are necessarily booleans, > and those are covered since they will never be '== mod', so their > visibility was, and still is, properly handled. > >> From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001 >> From: Dirk Gouders <dirk@gouders.net> >> Date: Wed, 30 Oct 2013 15:06:05 +0100 >> Subject: [PATCH 2/2] kconfig/symbol.c: handle visibility of choice_values that >> depend on 'm' symbols >> >> If choice_values depend on symbols that are set to 'm' then these >> choice_values should not be visible in choice lists if the choice >> symbol is set to 'y'. See USB Gadget Drivers, for example. > > I liked your previous commit message better, because it had a test-case > that did exhibit the problem. > > Can you please: > - confirm which patch/es is/are needed > - update the commit log/s back with the test-case/s > - resend needed patch/es Thanks for the comments, Yann, and apologies for the confusion. Patch v3 comes in a minute and hopefully in a satisfactory format. You are right, both patches that I sent fix the same (reported) problem but v2 seems to be more sensible to me, because it causes non-selectable choices to not even appear in choice lists. However, it uses the side-effect or "natural consequence" that a tristate choice_value's value is set to no in sym_calc_value() if it's visibility is no. So, this seems to be a bit subtle and I tried to address it in the new commit message. I probably noticed another problem with those tristate choices: if a non-optional tristate choice is set to 'm', then the default value is not selected if no other is and I think that is not correct, but I'd prefer to hear if others also think that this is a problem that should be fixed. Dirk -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 043c041..fbabb80 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym) static void sym_calc_visibility(struct symbol *sym) { struct property *prop; + struct symbol *choice_sym = NULL; tristate tri; /* any prompt visible? */ tri = no; + + if (sym_is_choice_value(sym)) + choice_sym = prop_get_symbol(sym_get_choice_prop(sym)); + for_all_prompts(sym, prop) { prop->visible.tri = expr_calc_value(prop->visible.expr); + /* + * choice_values with visibility 'mod' are not visible if the + * corresponding choice's value is 'yes'. + */ + if (prop->visible.tri == mod && (choice_sym != NULL && choice_sym->curr.tri == yes)) + prop->visible.tri = no; tri = EXPR_OR(tri, prop->visible.tri); } if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))