Re: choice =y selection becomes lost after having multiple entries =m with depends on

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Dirk Gouders <dirk@gouders.net>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Michal Marek <mmarek@suse.cz>,
	linux-kbuild@vger.kernel.org, Felipe Balbi <balbi@ti.com>,
	USB list <linux-usb@vger.kernel.org>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Roger Quadros <rogerq@ti.com>
Subject: Re: choice =y selection becomes lost after having multiple entries =m with depends on
Date: Thu, 31 Oct 2013 22:49:58 +0100	[thread overview]
Message-ID: <20131031214958.GA5260@free.fr> (raw)
In-Reply-To: <giob664yy1.fsf@karga.hank.lab>

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  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'