Re: [PATCH v3 4/6] tpm: Support TPM2 sized buffers (TPM2B)

[James Bottomley]

On Tue, 2023-10-24 at 04:15 +0300, Jarkko Sakkinen wrote:
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -7,22 +7,32 @@
>  #include <linux/tpm.h>
>  
>  /**
> - * tpm_buf_init() - Initialize from the heap
> + * tpm_buf_init() - Initialize a TPM buffer
>   * @buf:       A @tpm_buf
> + * @sized:     Represent a sized buffer (TPM2B)
> + * @alloc:     Allocate from the heap
>   *
>   * Initialize all structure fields to zero, allocate a page from the
> heap, and
>   * zero the bytes that the buffer headers will consume.
>   *
>   * Return: 0 or -ENOMEM
>   */
> -int tpm_buf_init(struct tpm_buf *buf)
> +int tpm_buf_init(struct tpm_buf *buf, bool alloc, bool sized)

I think it creates a phenomenally confusing interface to use multiple
booleans because, unlike flags, it's not self describing at point of
use.  The confusion is enormously heightened here by having the doc
book arguments be the reverse of the actual function prototype (I just
tripped over this).

The alloc flag is particularly counter intuitive: if you pass in an
allocated buffer, you expect to be responsible for freeing it again,
but that's not how you use it; you really use it like a reset not an
alloc, which looks odd because you already created a separate
tpm_buf_reset function which can't be used in this case.

Why not replace the alloc flags with two reset functions: one for TPM2B
buffers and one for command buffers?

James