On Fri, Oct 18, 2024 at 02:02:48PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > The signature check in of the formatting context is never getting
> > released. Fix this to plug the resulting memory leak.
> >
> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> > ---
> > pretty.c | 1 +
> > t/t4202-log.sh | 1 +
> > t/t7031-verify-tag-signed-ssh.sh | 1 +
> > t/t7510-signed-commit.sh | 1 +
> > t/t7528-signed-commit-ssh.sh | 1 +
> > 5 files changed, 5 insertions(+)
> >
> > diff --git a/pretty.c b/pretty.c
> > index 6403e268900..098378720a4 100644
> > --- a/pretty.c
> > +++ b/pretty.c
> > @@ -2032,6 +2032,7 @@ void repo_format_commit_message(struct repository *r,
> >
> > free(context.commit_encoding);
> > repo_unuse_commit_buffer(r, commit, context.message);
> > + signature_check_clear(&context.signature_check);
>
> I was having a very hard time finding where this gets allocated, and to
> be honest, I still don't know for sure. I think in
> check_commit_signature() which is called by format_commit_one().
> In "[PATCH 20/21] builtin/merge: release outbut buffer after performing
> merge" you mention it's not obvious to the caller they need know about
> memory they need to clean up, isn't that case here as well?
Well, I think it's clearer in this context, but hidden by the fact that
we pass around a wrapper-structure. But that's not really the problem of
the `struct signature_check` subsystem, but rather of "pretty.c".
In any case, the callchain is:
- `format_commit_one()`
- `check_commit_signature()`
- `check_signature()`
- `verify_signed_buffer()`, which is a function pointer depending on
whether we verify a GPG, x509 or SSH signature.
