Re: [PATCH 1/8] gpg-interface: handle bool user.signingkey

2018-04-10 Thread Jeff King
On Mon, Apr 09, 2018 at 04:55:26PM -0400, Eric Sunshine wrote:

> Peff's Signed-off-by: is missing. Also, since you're forwarding this
> patch on Peff's behalf, your Signed-off-by: should follow his. Same
> comment applies to all patches by Peff in this series.

I usually sign-off as I send to the list, but I passed these patches to
Ben via a repository. For the record, it's OK to add my s-o-b to the
whole series.

-Peff


Re: [PATCH 1/8] gpg-interface: handle bool user.signingkey

2018-04-09 Thread Eric Sunshine
On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews  wrote:
> From: Jeff King 
>
> The config handler for user.signingkey does not check for a
> boolean value, and thus:
>
>   git -c user.signingkey tag
>
> will segfault. We could fix this and even shorten the code
> by using git_config_string(). But our set_signing_key()
> helper is used by other code outside of gpg-interface.c, so
> we must keep it (and we may as well use it, because unlike
> git_config_string() it does not leak when we overwrite an
> old value).
>
> Ironically, the handler for gpg.program just below _could_
> use git_config_string() but doesn't. But since we're going
> to touch that in a future patch, we'll leave it alone for
> now. We will add some whitespace and returns in preparation
> for adding more config keys, though.
> ---

Peff's Signed-off-by: is missing. Also, since you're forwarding this
patch on Peff's behalf, your Signed-off-by: should follow his. Same
comment applies to all patches by Peff in this series.

The patch itself looks reasonable.

> diff --git a/gpg-interface.c b/gpg-interface.c
> index 4feacf16e5..61c0690e12 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -128,13 +128,19 @@ void set_signing_key(const char *key)
>  int git_gpg_config(const char *var, const char *value, void *cb)
>  {
> if (!strcmp(var, "user.signingkey")) {
> +   if (!value)
> +   return config_error_nonbool(var);
> set_signing_key(value);
> +   return 0;
> }
> +
> if (!strcmp(var, "gpg.program")) {
> if (!value)
> return config_error_nonbool(var);
> gpg_program = xstrdup(value);
> +   return 0;
> }
> +
> return 0;
>  }