On Thu, 2024-07-04 at 10:07 -0700, Linus Torvalds wrote:
On Wed, 3 Jul 2024 at 13:11, James Bottomley James.Bottomley@hansenpartnership.com wrote:
if (__and(IS_ENABLED(CONFIG_TCG_TPM2_HMAC), chip->auth))
Augh. Please don't do this.
That "__and()" thing may work, but it's entirely accidental that it does.
It's designed for config options _only_, and the fact that it then happens to work for "first argument is config option, second argument is C conditional".
The comment says that it's implementing "&&" using preprocessor expansion only, but it's a *really* limited form of it. The arguments are *not* arbitrary.
So no. Don't do this.
Just create a helper inline like
static inline struct tpm2_auth *chip_auth(struct tpm_chip *chip) { #ifdef CONFIG_TCG_TPM2_HMAC return chip->auth; #else return NULL; #endif }
and if we really want to have some kind of automatic way of doing this, we will *NOT* be using __and(), we'd do something like
/* Return zero or 'value' depending on whether OPTION is enabled or not */ #define IF_ENABLED(option, value) __and(IS_ENABLED(option), value)
that actually would be documented and meaningful.
Not this internal random __and() implementation that is purely a kconfig.h helper macro and SHOULD NOT be used anywhere else.
I actually like the latter version, but instinct tells me that if this is the first time the kernel has ever needed something like this then perhaps we should go with the former because that's how everyone must have handled it in the past.
James