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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20251209224548.3885246-1-grahamr@qti.qualcomm.com>
Date: Tue,  9 Dec 2025 14:45:48 -0800
From: Graham Roff <grahamr@....qualcomm.com>
To: nathan@...nel.org
Cc: arnd@...db.de, corbet@....net, grahamr@....qualcomm.com,
        linux-doc@...r.kernel.org, linux-kbuild@...r.kernel.org,
        linux-kernel@...r.kernel.org, nico@...xnic.net, nsc@...nel.org
Subject: Re: [PATCH v2] Support conditional deps using "depends on X if Y"

> Other than that, this seems reasonable to me. The actual code changes
> are small and the tests prove this works properly. I won't pick up v3
> until after 6.19-rc1 is out at the least.

Thanks Nathan! I will send out a v3 with the commit text updated, and the test
code changes you suggested.

> > > On the surface, the vast majority these become more readable using the
> > > 'if' syntax.
> >
> > Agreed, the question is whether a small improvement in
> > readability is worth the complexity of having multiple
> > ways of expressing the same thing.
>
> It is a tradeoff. Sometimes it is advantageous to increase the 
> complexity in one place so other areas with more exposure to more people 
> are simplified.

Exactly, a small code update here provides a much simpler syntax for 
expressing a fairly common thing (conditional or optional dependencies).

It also makes the language more consistent since most other kconfig commands
accept a trailing "if <expr>" - I just assumed that "depends on" did as well
when first writing Kconfig files!

> >> +config TEST_COMPLEX
> >> +    bool "Test complex conditional dependency"
> >> +    depends on (FOO && BAR) if (FOO || BAR)
> >> +    default y
> 
> With the existing syntax, this could be expressed as
> 
>       depends on FOO = BAR
> 
> or
> 
>       depends on (FOO && BAR) || (!FOO && !BAR)
> 
> and I don't see how the new syntax is an improvement
> over these.

In this case I agree, the test was chosen to validate a complicated 
conditional, not to show a particularly useful real-world example.
I added much better examples earlier of where the "if" style provides
improved readability.
 
> Overall, I'm not convinced by this patch. I have no strong
> objection to anything in here, but I'm worried that extending
> the syntax adds more problems than this one solves.

Both syntaxes have their places, it comes down to which one is most
understandable to the reader. In a lot of cases it is definitely
easier to understand the intent of the expression using "if" rather
than ors and nots. Combine that with the simplicity of the change
and any problems this might add (not sure what those are though)
would seem on balance to be worth it.

Graham






Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ