Re: [PATCH v2 2/3] tpm: Address !chip->auth in tpm_buf_append_name()

Linus Torvalds
2024-07-04T17:08:15Z

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.

Linus