All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-kernel@vger.kernel.org, keyrings@vger.kernel.org,
	Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>,
	Peter Huewe <peterhuewe@gmx.de>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	David Howells <dhowells@redhat.com>,
	Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"open list:TPM DEVICE DRIVER" <linux-integrity@vger.kernel.org>,
	"open list:SECURITY SUBSYSTEM"
	<linux-security-module@vger.kernel.org>
Subject: Re: [PATCH] tpm: Create cleanup class for tpm_buf
Date: Fri, 27 Jun 2025 01:33:04 +0300	[thread overview]
Message-ID: <aF3KoHdo6dY6nYbw@kernel.org> (raw)
In-Reply-To: <aF2QbDmxzGJS903j@kernel.org>

On Thu, Jun 26, 2025 at 09:24:48PM +0300, Jarkko Sakkinen wrote:
> > At least I would add the class and drop the tpm_buf_destroy() as one
> > patch, and another would be to cleanup any empty gotos.
> > 
> > Also, I think the style guide for cleanup.h is to not use the
> > variable block, so it should be more like:
> > 
> > CLASS(tpm_buf, buf)();
> > if (!tpm_buf)
> >    return -ENOMEM;
> > 
> > AFAICT, but that seems to be some kind of tribal knowledge.
> 
> This was improved in v2 :-) If you have some proposal how you'd
> liked that version to be splitted, please give feedback.

After a bit of thought, II could split v2 e.g., into to the following
list of patches (a draft, along the lines):

1. Prepare internals for API changes.
2. Implement tpm_buf_alloc().
3. Implement CLASS_TPM_BUF() macro.
4. Changes for tpm{1,2}-cmd.c.
6. Changes for tpm2-sessions.c.
7. Changes for tpm2-space.c.
8. Changes for trusted_tpm{1,2}.c
9. Remove stuff left w/o a call site.

It's pretty good exercise for v2 actually as it is already somewhat
functional code. By doing this split this update will get already
reasonably well verified.

I should also probably emphasize the motivation better in the next
version.  Especially with multiple tpm_buf instances in the same
function scope, things do something are messy to backtrack. In addition,
this complexity might cap the motivation for someone to contribute a
useful feature.

I don't really have even followed Linus' opinions in this topic per se
I personally just think that since I have a measured argument for this.
I got with that and talk with Linus if he wants to bring it up :-)

BR, Jarkko

      reply	other threads:[~2025-06-26 22:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-25 21:37 [PATCH] tpm: Create cleanup class for tpm_buf Jarkko Sakkinen
2025-06-25 21:45 ` Jarkko Sakkinen
2025-06-26 14:49 ` Jason Gunthorpe
2025-06-26 18:24   ` Jarkko Sakkinen
2025-06-26 22:33     ` Jarkko Sakkinen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aF3KoHdo6dY6nYbw@kernel.org \
    --to=jarkko@kernel.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=dhowells@redhat.com \
    --cc=jarkko.sakkinen@opinsys.com \
    --cc=jgg@ziepe.ca \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=peterhuewe@gmx.de \
    --cc=serge@hallyn.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.