On Fri, 2023-10-27 at 08:32 -0400, James Bottomley wrote: > 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 Or you can make that as internal (__tpm_buf_init()) and add two wrappers. BR, Jarkko
