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

[Junio C Hamano <gitster@pobox.com>]

Justin Tobler <jltobler@gmail.com> writes:

> The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
> logic to get the default configured signing key when a key is not
> provided and handles generating the commit signature accordingly. This
> signing operation is not really specific to commits as any arbitrary
> buffer can be signed. Also, in a subsequent commit, this same logic is
> reused by git-fast-import(1) when re-signing invalid commit signatures.
>
> Move the `sign_commit_to_strbuf()` helper from "commit.c" to
> "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
> this function so it can be used by "commit.c" and
> "builtin/fast-import.c" in the subsequent commit.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  commit.c        | 16 ++--------------
>  gpg-interface.c | 13 +++++++++++++
>  gpg-interface.h |  7 +++++++
>  3 files changed, 22 insertions(+), 14 deletions(-)

Sennsible restructuring that makes the machinery easier to reuse.
Updated function is named more appropriately for public consumption.

Overall a very welcome preparation step.

> diff --git a/commit.c b/commit.c
> index d16ae73345..1677b1ef25 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
>  	return 0;
>  }
>  
> -static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
> -{
> -	char *keyid_to_free = NULL;
> -	int ret = 0;
> -	if (!keyid || !*keyid)
> -		keyid = keyid_to_free = get_signing_key();
> -	if (sign_buffer(buf, sig, keyid))
> -		ret = -1;
> -	free(keyid_to_free);
> -	return ret;
> -}
> -
>  int parse_signed_commit(const struct commit *commit,
>  			struct strbuf *payload, struct strbuf *signature,
>  			const struct git_hash_algo *algop)
> @@ -1737,7 +1725,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		oidcpy(&parent_buf[i++], &p->item->object.oid);
>  
>  	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
> -	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
> +	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
>  		result = -1;
>  		goto out;
>  	}
> @@ -1769,7 +1757,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		free_commit_extra_headers(compat_extra);
>  		free(mapped_parents);
>  
> -		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
> +		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
>  			result = -1;
>  			goto out;
>  		}
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 87fb6605fb..a72fa35061 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -980,6 +980,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	return use_format->sign_buffer(buffer, signature, signing_key);
>  }
>  
> +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> +			 const char *signing_key)
> +{
> +	char *keyid_to_free = NULL;
> +	int ret = 0;
> +	if (!signing_key || !*signing_key)
> +		signing_key = keyid_to_free = get_signing_key();
> +	if (sign_buffer(buffer, signature, signing_key))
> +		ret = -1;
> +	free(keyid_to_free);
> +	return ret;
> +}
> +
>  /*
>   * Strip CR from the line endings, in case we are on Windows.
>   * NEEDSWORK: make it trim only CRs before LFs and rename
> 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);
>  
>  /*
>   * Returns corresponding string in lowercase for a given member of