"Yann E. MORIN" <yann.morin.1998@xxxxxxx> writes: > Dirk, All, > > On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly: >> Dirk Gouders <dirk@xxxxxxxxxxx> 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@xxxxxxxxxxx> >> 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-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
