Web lists-archives.com

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




On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews <mastahyeti@xxxxxxxxx> wrote:
> From: Jeff King <peff@xxxxxxxx>
>
> 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;
>  }