Subject: Re: [PATCH v4 2/3] gpg-interface: introduce sign_buffer_with_key()

From: Justin Tobler <[email-masked]>
Date: Thu, 12 Mar 2026 08:58:06 -0500

On 26/03/12 11:22AM, Patrick Steinhardt wrote:
> On Wed, Mar 11, 2026 at 12:31:46PM -0500, Justin Tobler wrote:
> > diff --git a/gpg-interface.h b/gpg-interface.h
> > index 789d1ffac4..a32741aeda 100644
> > --- a/gpg-interface.h
> > +++ b/gpg-interface.h
> > @@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
> >  int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
> >  		const char *signing_key);
> >  
> > +/*
> > + * Similar to `sign_buffer()`, but uses the default configured signing key as
> > + * returned by `get_signing_key()` when the provided "signing_key" is NULL or
> > + * empty. Returns 0 on success, non-zero on failure.
> > + */
> > +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> > +			 const char *signing_key);
> 
> I think this interface is a bit confusing, as you wouldn't really be
> able to tell what the difference between `sign_buffer()` and
> `sign_buffer_with_key()` is without having a deeper look. Naively, I
> would expect the latter function to be the one that actually mandates
> that the user provides a key, but it's the other way round.
> 
> Would it be preferable to instead extend `sign_buffer()` to take a flags
> parameter and then introduce `SIGN_BUFFER_USE_DEFAULT_KEY` to make it
> fall back to the configured signing key? If so, we could drop
> `sign_commit_to_strbuf()` completely.

That's fair, and this suggestion sounds completely sensible to me. There
are only a handful to `sign_buffer()` callers, so it should create too
much churn either. Will send another version adapted accordingly.

-Justin