push: drop confusing configset/callback redundancy

We parse push config by calling git_config() with our git_push_config()
callback. But inside that callback, when we see "push.gpgsign", we
ignore the value passed into the callback and instead make a new call to
git_config_get_value().

This is unnecessary at best, and slightly wrong at worst (if there are
multiple instances, get_value() only returns one; both methods end up
with last-one-wins, but we'd fail to report errors if earlier
incarnations were bogus).

The call was added by 68c757f219 (push: add a config option push.gpgSign
for default signed pushes, 2015-08-19). That commit doesn't give any
reason to deviate from the usual strategy here; it was probably just
somebody unfamiliar with our config API and its conventions.

It also added identical code to builtin/send-pack.c, which also handles
push.gpgsign.

And then the same issue spread to its neighbor in b33a15b081 (push: add
recurseSubmodules config option, 2015-11-17), presumably via
cargo-culting.

This patch fixes all three to just directly use the value provided to
the callback. While I was adjusting the code to do so, I noticed that
push.gpgsign is overly careful about a NULL value. After
git_parse_maybe_bool() has returned anything besides 1, we know that the
value cannot be NULL (if it were, it would be an implicit "true", and
many callers of maybe_bool rely on that). Here that lets us shorten "if
(v && !strcasecmp(v, ...))" to just "if (!strcasecmp(v, ...))".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/push.c b/builtin/push.c
index 2e708383c24ba86e07fc14b0139afb28d489c014..58a9273e90358a6253cfbc20b4467954307ba688 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -526,26 +526,21 @@ static int git_push_config(const char *k, const char *v,
                        *flags |= TRANSPORT_PUSH_AUTO_UPSTREAM;
                return 0;
        } else if (!strcmp(k, "push.gpgsign")) {
-               const char *value;
-               if (!git_config_get_value("push.gpgsign", &value)) {
-                       switch (git_parse_maybe_bool(value)) {
-                       case 0:
-                               set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_NEVER);
-                               break;
-                       case 1:
-                               set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_ALWAYS);
-                               break;
-                       default:
-                               if (value && !strcasecmp(value, "if-asked"))
-                                       set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_IF_ASKED);
-                               else
-                                       return error(_("invalid value for '%s'"), k);
-                       }
+               switch (git_parse_maybe_bool(v)) {
+               case 0:
+                       set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_NEVER);
+                       break;
+               case 1:
+                       set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_ALWAYS);
+                       break;
+               default:
+                       if (!strcasecmp(v, "if-asked"))
+                               set_push_cert_flags(flags, SEND_PACK_PUSH_CERT_IF_ASKED);
+                       else
+                               return error(_("invalid value for '%s'"), k);
                }
        } else if (!strcmp(k, "push.recursesubmodules")) {
-               const char *value;
-               if (!git_config_get_value("push.recursesubmodules", &value))
-                       recurse_submodules = parse_push_recurse_submodules_arg(k, value);
+               recurse_submodules = parse_push_recurse_submodules_arg(k, v);
        } else if (!strcmp(k, "submodule.recurse")) {
                int val = git_config_bool(k, v) ?
                        RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index cd6d9e41129a21d7f9c21cebae0a322812ebcbcd..00e6c90477be689df85cddf606be772b74709e2b 100644 (file)
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -135,21 +135,18 @@ static int send_pack_config(const char *k, const char *v,
                            const struct config_context *ctx, void *cb)
 {
        if (!strcmp(k, "push.gpgsign")) {
-               const char *value;
-               if (!git_config_get_value("push.gpgsign", &value)) {
-                       switch (git_parse_maybe_bool(value)) {
-                       case 0:
-                               args.push_cert = SEND_PACK_PUSH_CERT_NEVER;
-                               break;
-                       case 1:
-                               args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
-                               break;
-                       default:
-                               if (value && !strcasecmp(value, "if-asked"))
-                                       args.push_cert = SEND_PACK_PUSH_CERT_IF_ASKED;
-                               else
-                                       return error(_("invalid value for '%s'"), k);
-                       }
+               switch (git_parse_maybe_bool(v)) {
+               case 0:
+                       args.push_cert = SEND_PACK_PUSH_CERT_NEVER;
+                       break;
+               case 1:
+                       args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
+                       break;
+               default:
+                       if (!strcasecmp(v, "if-asked"))
+                               args.push_cert = SEND_PACK_PUSH_CERT_IF_ASKED;
+                       else
+                               return error(_("invalid value for '%s'"), k);
                }
        }
        return git_default_config(k, v, ctx, cb);