config: handle NULL value when parsing non-bools

When the config parser sees an "implicit" bool like:

  [core]
  someVariable

it passes NULL to the config callback. Any callback code which expects a
string must check for NULL. This usually happens via helpers like
git_config_string(), etc, but some custom code forgets to do so and will
segfault.

These are all fairly vanilla cases where the solution is just the usual
pattern of:

  if (!value)
        return config_error_nonbool(var);

though note that in a few cases we have to split initializers like:

  int some_var = initializer();

into:

  int some_var;
  if (!value)
        return config_error_nonbool(var);
  some_var = initializer();

There are still some broken instances after this patch, which I'll
address on their own in individual patches after this one.

Reported-by: Carlos Andrés Ramírez Cataño <antaigroupltda@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/blame.c b/builtin/blame.c
index 9c987d656756e8f436e3b1cb818306097e402e4c..2433b7da5cec6b8300b0316d8054af486a92ba7f 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -748,6 +748,8 @@ static int git_blame_config(const char *var, const char *value,
        }
 
        if (!strcmp(var, "blame.coloring")) {
+               if (!value)
+                       return config_error_nonbool(var);
                if (!strcmp(value, "repeatedLines")) {
                        coloring_mode |= OUTPUT_COLOR_LINE;
                } else if (!strcmp(value, "highlightRecent")) {

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f02434bc155ba1769a248350afa686322e6110bb..d5c784854fb621c188bd4b40a29b5bbf95b6e884 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1202,6 +1202,8 @@ static int git_checkout_config(const char *var, const char *value,
        struct checkout_opts *opts = cb;
 
        if (!strcmp(var, "diff.ignoresubmodules")) {
+               if (!value)
+                       return config_error_nonbool(var);
                handle_ignore_submodules_arg(&opts->diff_options, value);
                return 0;
        }

diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af949895a688639c83984598931906b2690..54d9b9976aa62c36e54c0773b2169ba9c7228779 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -791,6 +791,8 @@ static int git_clone_config(const char *k, const char *v,
                            const struct config_context *ctx, void *cb)
 {
        if (!strcmp(k, "clone.defaultremotename")) {
+               if (!v)
+                       return config_error_nonbool(k);
                free(remote_name);
                remote_name = xstrdup(v);
        }

diff --git a/builtin/log.c b/builtin/log.c
index ba775d7b5cf886374bd94c9f2e54d5695a3dbaff..3ce41c48568dca8a6562293807c5595929b36e73 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -594,8 +594,11 @@ static int git_log_config(const char *var, const char *value,
                        decoration_style = 0; /* maybe warn? */
                return 0;
        }
-       if (!strcmp(var, "log.diffmerges"))
+       if (!strcmp(var, "log.diffmerges")) {
+               if (!value)
+                       return config_error_nonbool(var);
                return diff_merges_config(value);
+       }
        if (!strcmp(var, "log.showroot")) {
                default_show_root = git_config_bool(var, value);
                return 0;

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 89a8b5a9768e42da3edaaddb63a663936ef70576..62c540b4db3ae4c334cb6709c1b5505e3d50912b 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3204,7 +3204,7 @@ static int git_pack_config(const char *k, const char *v,
                return 0;
        }
        if (!strcmp(k, "uploadpack.blobpackfileuri")) {
-               struct configured_exclusion *ex = xmalloc(sizeof(*ex));
+               struct configured_exclusion *ex;
                const char *oid_end, *pack_end;
                /*
                 * Stores the pack hash. This is not a true object ID, but is
@@ -3212,6 +3212,10 @@ static int git_pack_config(const char *k, const char *v,
                 */
                struct object_id pack_hash;
 
+               if (!v)
+                       return config_error_nonbool(k);
+
+               ex = xmalloc(sizeof(*ex));
                if (parse_oid_hex(v, &ex->e.oid, &oid_end) ||
                    *oid_end != ' ' ||
                    parse_oid_hex(oid_end + 1, &pack_hash, &pack_end) ||

diff --git a/compat/mingw.c b/compat/mingw.c
index ec5280da160135170e2d778cfcb19cea1752c211..42053c1f656bd8b73696b751f30177ce9e544d59 100644 (file)
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -255,6 +255,8 @@ int mingw_core_config(const char *var, const char *value,
        }
 
        if (!strcmp(var, "core.unsetenvvars")) {
+               if (!value)
+                       return config_error_nonbool(var);
                free(unset_environment_variables);
                unset_environment_variables = xstrdup(value);
                return 0;

diff --git a/config.c b/config.c
index b330c7adb4a5efc3cac898391d4c722b019d95a1..18085c7e387a756f4039776cc63fba5f56b684f6 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1386,6 +1386,8 @@ static int git_default_core_config(const char *var, const char *value,
                return 0;
        }
        if (!strcmp(var, "core.checkstat")) {
+               if (!value)
+                       return config_error_nonbool(var);
                if (!strcasecmp(value, "default"))
                        check_stat = 1;
                else if (!strcasecmp(value, "minimal"))
@@ -1547,11 +1549,15 @@ static int git_default_core_config(const char *var, const char *value,
        }
 
        if (!strcmp(var, "core.checkroundtripencoding")) {
+               if (!value)
+                       return config_error_nonbool(var);
                check_roundtrip_encoding = xstrdup(value);
                return 0;
        }
 
        if (!strcmp(var, "core.notesref")) {
+               if (!value)
+                       return config_error_nonbool(var);
                notes_ref_name = xstrdup(value);
                return 0;
        }
@@ -1619,6 +1625,8 @@ static int git_default_core_config(const char *var, const char *value,
        }
 
        if (!strcmp(var, "core.createobject")) {
+               if (!value)
+                       return config_error_nonbool(var);
                if (!strcmp(value, "rename"))
                        object_creation_mode = OBJECT_CREATION_USES_RENAMES;
                else if (!strcmp(value, "link"))

diff --git a/diff.c b/diff.c
index 2c602df10a372c7e99402ca5a2dc9cf7aa673c09..5b213a4b44e96d76ce218f801627e313ace7b17b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -372,7 +372,10 @@ int git_diff_ui_config(const char *var, const char *value,
                return 0;
        }
        if (!strcmp(var, "diff.colormovedws")) {
-               unsigned cm = parse_color_moved_ws(value);
+               unsigned cm;
+               if (!value)
+                       return config_error_nonbool(var);
+               cm = parse_color_moved_ws(value);
                if (cm & COLOR_MOVED_WS_ERROR)
                        return -1;
                diff_color_moved_ws_default = cm;
@@ -426,10 +429,15 @@ int git_diff_ui_config(const char *var, const char *value,
        if (!strcmp(var, "diff.orderfile"))
                return git_config_pathname(&diff_order_file_cfg, var, value);
 
-       if (!strcmp(var, "diff.ignoresubmodules"))
+       if (!strcmp(var, "diff.ignoresubmodules")) {
+               if (!value)
+                       return config_error_nonbool(var);
                handle_ignore_submodules_arg(&default_diff_options, value);
+       }
 
        if (!strcmp(var, "diff.submodule")) {
+               if (!value)
+                       return config_error_nonbool(var);
                if (parse_submodule_params(&default_diff_options, value))
                        warning(_("Unknown value for 'diff.submodule' config variable: '%s'"),
                                value);
@@ -473,7 +481,10 @@ int git_diff_basic_config(const char *var, const char *value,
        }
 
        if (!strcmp(var, "diff.wserrorhighlight")) {
-               int val = parse_ws_error_highlight(value);
+               int val;
+               if (!value)
+                       return config_error_nonbool(var);
+               val = parse_ws_error_highlight(value);
                if (val < 0)
                        return -1;
                ws_error_highlight_default = val;
@@ -490,6 +501,8 @@ int git_diff_basic_config(const char *var, const char *value,
 
        if (!strcmp(var, "diff.dirstat")) {
                struct strbuf errmsg = STRBUF_INIT;
+               if (!value)
+                       return config_error_nonbool(var);
                default_diff_options.dirstat_permille = diff_dirstat_permille_default;
                if (parse_dirstat_params(&default_diff_options, value, &errmsg))
                        warning(_("Found errors in 'diff.dirstat' config variable:\n%s"),

diff --git a/mailinfo.c b/mailinfo.c
index a07d2da16dedb2779a9797cfa185ec29d9321491..093bed5d8fb62a30e5fe3febefb6a8ff4508424a 100644 (file)
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1253,6 +1253,8 @@ static int git_mailinfo_config(const char *var, const char *value,
                return 0;
        }
        if (!strcmp(var, "mailinfo.quotedcr")) {
+               if (!value)
+                       return config_error_nonbool(var);
                if (mailinfo_parse_quoted_cr_action(value, &mi->quoted_cr) != 0)
                        return error(_("bad action '%s' for '%s'"), value, var);
                return 0;

diff --git a/notes-utils.c b/notes-utils.c
index 97c031c26ec7c7c2fdade8478fa039d470820bad..01f4f5b42415b974b806850da130bf1f6c8ec4e0 100644 (file)
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -112,6 +112,8 @@ static int notes_rewrite_config(const char *k, const char *v,
                }
                return 0;
        } else if (!c->refs_from_env && !strcmp(k, "notes.rewriteref")) {
+               if (!v)
+                       return config_error_nonbool(k);
                /* note that a refs/ prefix is implied in the
                 * underlying for_each_glob_ref */
                if (starts_with(v, "refs/notes/"))

diff --git a/trailer.c b/trailer.c
index b6de5d9cb2dab2c07c26775e62e5bfc40241c9fc..b0e2ec224a2fa16ab0f87012a5b0a4bf90996760 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -507,6 +507,8 @@ static int git_trailer_default_config(const char *conf_key, const char *value,
                                warning(_("unknown value '%s' for key '%s'"),
                                        value, conf_key);
                } else if (!strcmp(trailer_item, "separators")) {
+                       if (!value)
+                               return config_error_nonbool(conf_key);
                        separators = xstrdup(value);
                }
        }