From patchwork Wed Oct 26 15:35:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020832 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05B97FA3741 for ; Wed, 26 Oct 2022 15:36:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234600AbiJZPgn (ORCPT ); Wed, 26 Oct 2022 11:36:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59108 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234588AbiJZPgk (ORCPT ); Wed, 26 Oct 2022 11:36:40 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE2E466851 for ; Wed, 26 Oct 2022 08:36:37 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id a14so24218785wru.5 for ; Wed, 26 Oct 2022 08:36:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vbnBu4Z7IZWoxSwdRcAgYT5+HbG75DrYjaXhpNKvJkY=; b=AkSgs6P23Vgr0E4pdNaLuu6QV4TV7EIG4E2fWGV6U4dU7L/OfBSModJZtdrnPOnM6Y mmRjRbdVV3pv+wadxewVFhAC/PZCI66qye70u/riW87s2WaVOvL+fq93gK/Ei2jonO3c oBBikk7f1ofNY6wmdbhfVFCGB6RMSl0553MmM8fltVyql/hSpLcDvMB2aTX0TzJz7T8A 79MjZaZ+QqZ+STb4L5fZ/+JOLWMQ/0XVDDBTvCnhxKRU3PpmdtsJQCHSOUrCKQt7BfuH 8i4BLyl8Vq/RjGH6fPKNjymh6kbb0KXEinfrRC57MTOJR8tcK8g2QETy5eB8QyDo7soV +WdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vbnBu4Z7IZWoxSwdRcAgYT5+HbG75DrYjaXhpNKvJkY=; b=hhpcEWztkoEP3X408w6Y9mxBxm97zHUDkWYMa4H1nUJ74uZzF9oQRH0FoThhcgUuRJ Yew9pu9AY1npCAngrltHKuq3edFH1V7x49Aj18x+vtXwxOvLLjPXxyumliIx3NVQecaS ufZA9wTlPbvEyyC2ASIkZLcsZ3WjGZe0hgUvw2Eoi94ERYV3kB+/ANh8Py/vLuOuZfoK xRuDK/pu2UYNsYFmhzCtrncUbdZaQp0pF5RQCag4mWlv0UeEx1OXSXl1ZynY3qTgMTVj OiiQQuD5RmTB/lfzWF1OPJNw8Qy4w+Hmb7q4FV7ddz8n5jFSLrDzkrSdm3QOCdD6Bg0a 1iKg== X-Gm-Message-State: ACrzQf3JDotvaKxYhEFaIZ+vGYfrQPwLMuCcLXnDiRje/MOEYpEYdMX9 L1MuhmuBvbHd7Oawsvq9glPqikZtm3gu2g== X-Google-Smtp-Source: AMsMyM6TrEO0/ArzNGOReMOD2wweOywvG3GiGxHnUIfGkec3dSqrg2j4UR6QcQ4Me2hako1/8jOtGg== X-Received: by 2002:a05:6000:1f1a:b0:236:8ef6:472d with SMTP id bv26-20020a0560001f1a00b002368ef6472dmr1871330wrb.61.1666798595863; Wed, 26 Oct 2022 08:36:35 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:34 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 01/10] config API: have *_multi() return an "int" and take a "dest" Date: Wed, 26 Oct 2022 17:35:14 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The git_configset_get_value_multi() function added in 3c8687a73ee (add `config_set` API for caching config-like files, 2014-07-28) is a fundamental part of of the config API, and e.g. "git_config_get_value()" and others are implemented in terms of it. But it has had the limitation that configset_find_element() calls git_config_parse_key(), but then throws away the distinction between a "ret < 1" return value from it, and return values that indicate a key doesn't exist. As a result the git_config_get_value_multi() function would either return a "const struct string_list *", or NULL. By changing the *_multi() function to return an "int" for the status and to write to a "const struct string_list **dest" parameter we can avoid losing this information. API callers can now do: const struct string_list *dest; int ret; ret = git_config_get_value_multi(key, &dest); if (ret < 1) die("bad key: %s", key); else if (ret) ; /* key does not exist */ else ; /* got key, can use "dest" */ A "get_knownkey_value_multi" variant is also provided, which will BUG() out in the "ret < 1" case. This is useful in the cases where we hardcode the keyname in our source code, and therefore use the more idiomatic pattern of: if (!git_config_get_value_multi(key, &dest) ; /* got key, can use "dest" */ else ; /* key does not exist */ The "knownkey" name was picked instead of e.g. "const" to avoid a repeat of the issues noted in f1de981e8b6 (config: fix leaks from git_config_get_string_const(), 2020-08-14) and 9a53219f69b (config: drop git_config_get_string_const(), 2020-08-17). API users might think that "const" means that the value(s) don't need to be free'd. As noted in commentary here we treat git_die_config() as a special-case, i.e. we assume that a value we're complaining about has already had its key pass the git_config_parse_key() check. Likewise we consider the keys passed to "t/helper/test-config.c" to be "knownkey", and will emit a BUG() if they don't pass git_config_parse_key(). Those will come from our *.sh tests, so they're also "known keys" coming from our sources. A logical follow-up to this would be to change the various "*_get_*()" functions to ferry the git_configset_get_value() return value to their own callers, e.g.: diff --git a/config.c b/config.c index 094ad899e0b..7e8ee4cfec1 100644 --- a/config.c +++ b/config.c @@ -2479,11 +2479,14 @@ static int git_configset_get_string_tmp(struct config_set *cs, const char *key, int git_configset_get_int(struct config_set *cs, const char *key, int *dest) { const char *value; - if (!git_configset_get_value(cs, key, &value)) { - *dest = git_config_int(key, value); - return 0; - } else - return 1; + int ret; + + if ((ret = git_configset_get_value(cs, key, &value))) + goto done; + + *dest = git_config_int(key, value); +done: + return ret; } int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest) Most of those callers don't care, and call those functions as "if (!func(...))", but if they do they'll be able to tell key non-existence from errors we encounter. Before this change those API users would have been unable to tell the two conditions apart, as git_configset_get_value() hid the difference. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 5 +- builtin/gc.c | 6 +-- builtin/log.c | 6 +-- builtin/submodule--helper.c | 6 ++- config.c | 94 ++++++++++++++++++++++++++++++------- config.h | 52 ++++++++++++++++---- pack-bitmap.c | 7 ++- submodule.c | 3 +- t/helper/test-config.c | 6 +-- versioncmp.c | 10 ++-- 10 files changed, 148 insertions(+), 47 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index fd86e5a8619..b01721762ef 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -28,7 +28,7 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) { static const char *config_key = NULL; int i, result = 0; - const struct string_list *values; + const struct string_list *values = NULL; const struct option options[] = { OPT_STRING(0, "config", &config_key, N_("config"), @@ -42,8 +42,7 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) if (!config_key) die(_("missing --config=")); - values = repo_config_get_value_multi(the_repository, - config_key); + repo_config_get_value_multi(the_repository, config_key, &values); /* * Do nothing on an empty list, which is equivalent to the case diff --git a/builtin/gc.c b/builtin/gc.c index 243ee85d283..04c48638ef4 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1485,8 +1485,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) else git_config_set("maintenance.strategy", "incremental"); - list = git_config_get_value_multi(key); - if (list) { + if (!git_config_get_knownkey_value_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1542,8 +1541,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi usage_with_options(builtin_maintenance_unregister_usage, options); - list = git_config_get_value_multi(key); - if (list) { + if (!git_config_get_knownkey_value_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; diff --git a/builtin/log.c b/builtin/log.c index ee19dc5d450..75464c96ccf 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -182,10 +182,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f int i; char *value = NULL; struct string_list *include = decoration_filter->include_ref_pattern; - const struct string_list *config_exclude = - git_config_get_value_multi("log.excludeDecoration"); + const struct string_list *config_exclude; - if (config_exclude) { + if (!git_config_get_knownkey_value_multi("log.excludeDecoration", + &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) string_list_append(decoration_filter->exclude_ref_config_pattern, diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 0b4acb442b2..1f8fe6a8e0d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -541,6 +541,7 @@ static int module_init(int argc, const char **argv, const char *prefix) NULL }; int ret = 1; + const struct string_list *values; argc = parse_options(argc, argv, prefix, module_init_options, git_submodule_helper_usage, 0); @@ -552,7 +553,7 @@ static int module_init(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && git_config_get_value_multi("submodule.active")) + if (!argc && !git_config_get_value_multi("submodule.active", &values)) module_list_active(&list); info.prefix = prefix; @@ -2708,6 +2709,7 @@ static int module_update(int argc, const char **argv, const char *prefix) if (opt.init) { struct module_list list = MODULE_LIST_INIT; struct init_cb info = INIT_CB_INIT; + const struct string_list *values; if (module_list_compute(argc, argv, opt.prefix, &pathspec2, &list) < 0) { @@ -2720,7 +2722,7 @@ static int module_update(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && git_config_get_value_multi("submodule.active")) + if (!argc && !git_config_get_value_multi("submodule.active", &values)) module_list_active(&list); info.prefix = opt.prefix; diff --git a/config.c b/config.c index cbb5a3bab74..2100b29b689 100644 --- a/config.c +++ b/config.c @@ -2275,23 +2275,28 @@ void read_very_early_config(config_fn_t cb, void *data) config_with_options(cb, data, NULL, &opts); } -static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) +static int configset_find_element(struct config_set *cs, const char *key, + struct config_set_element **dest) { struct config_set_element k; struct config_set_element *found_entry; char *normalized_key; + int ret; + /* * `key` may come from the user, so normalize it before using it * for querying entries from the hashmap. */ - if (git_config_parse_key(key, &normalized_key, NULL)) - return NULL; + ret = git_config_parse_key(key, &normalized_key, NULL); + if (ret < 0) + return ret; hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; found_entry = hashmap_get_entry(&cs->config_hash, &k, ent, NULL); free(normalized_key); - return found_entry; + *dest = found_entry; + return 0; } static int configset_add_value(struct config_set *cs, const char *key, const char *value) @@ -2300,8 +2305,11 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha struct string_list_item *si; struct configset_list_item *l_item; struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); + int ret; - e = configset_find_element(cs, key); + ret = configset_find_element(cs, key, &e); + if (ret < 0) + return ret; /* * Since the keys are being fed by git_config*() callback mechanism, they * are already normalized. So simply add them without any further munging. @@ -2400,24 +2408,54 @@ int git_configset_add_parameters(struct config_set *cs) int git_configset_get_value(struct config_set *cs, const char *key, const char **value) { const struct string_list *values = NULL; + int ret; + /* * Follows "last one wins" semantic, i.e., if there are multiple matches for the * queried key in the files of the configset, the value returned will be the last * value in the value list for that key. */ - values = git_configset_get_value_multi(cs, key); + ret = git_configset_get_value_multi(cs, key, &values); - if (!values) + if (ret < 0) + return ret; + else if (!values) return 1; assert(values->nr > 0); *value = values->items[values->nr - 1].string; return 0; } -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key) +static int git_configset_get_value_multi_1(struct config_set *cs, const char *key, + const struct string_list **dest, + int knownkey) { - struct config_set_element *e = configset_find_element(cs, key); - return e ? &e->value_list : NULL; + struct config_set_element *e; + int ret; + + ret = configset_find_element(cs, key, &e); + if (ret < 0 && knownkey) + BUG("*_get_knownkey_*() only accepts known-good (hardcoded) keys, but '%s' is bad!", key); + else if (ret < 0) + return ret; + else if (!e) + return 1; + *dest = &e->value_list; + + return 0; +} + +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest) +{ + return git_configset_get_value_multi_1(cs, key, dest, 0); +} + +int git_configset_get_knownkey_value_multi(struct config_set *cs, + const char *const key, + const struct string_list **dest) +{ + return git_configset_get_value_multi_1(cs, key, dest, 1); } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -2563,11 +2601,20 @@ int repo_config_get_value(struct repository *repo, return git_configset_get_value(repo->config, key, value); } -const struct string_list *repo_config_get_value_multi(struct repository *repo, - const char *key) +int repo_config_get_value_multi(struct repository *repo, + const char *key, + const struct string_list **dest) { git_config_check_init(repo); - return git_configset_get_value_multi(repo->config, key); + return git_configset_get_value_multi(repo->config, key, dest); +} + +int repo_config_get_knownkey_value_multi(struct repository *repo, + const char *const key, + const struct string_list **dest) +{ + git_config_check_init(repo); + return git_configset_get_knownkey_value_multi(repo->config, key, dest); } int repo_config_get_string(struct repository *repo, @@ -2684,9 +2731,15 @@ int git_config_get_value(const char *key, const char **value) return repo_config_get_value(the_repository, key, value); } -const struct string_list *git_config_get_value_multi(const char *key) +int git_config_get_value_multi(const char *key, const struct string_list **dest) +{ + return repo_config_get_value_multi(the_repository, key, dest); +} + +int git_config_get_knownkey_value_multi(const char *const key, + const struct string_list **dest) { - return repo_config_get_value_multi(the_repository, key); + return repo_config_get_knownkey_value_multi(the_repository, key, dest); } int git_config_get_string(const char *key, char **dest) @@ -2833,7 +2886,16 @@ void git_die_config(const char *key, const char *err, ...) error_fn(err, params); va_end(params); } - values = git_config_get_value_multi(key); + + /* + * We don't have a "const" key here, but we should definitely + * have one that's passed git_config_parse_key() already, if + * we're at the point of complaining about its value. So let's + * use *_knownkey_value_multi() here to get that BUG(...). + */ + if (git_config_get_knownkey_value_multi(key, &values)) + BUG("key '%s' does not exist, should not be given to git_die_config()", + key); kv_info = values->items[values->nr - 1].util; git_die_config_linenr(key, kv_info->filename, kv_info->linenr); } diff --git a/config.h b/config.h index ca994d77147..c88619b7dcf 100644 --- a/config.h +++ b/config.h @@ -457,11 +457,30 @@ int git_configset_add_parameters(struct config_set *cs); /** * Finds and returns the value list, sorted in order of increasing priority - * for the configuration variable `key` and config set `cs`. When the - * configuration variable `key` is not found, returns NULL. The caller - * should not free or modify the returned pointer, as it is owned by the cache. + * for the configuration variable `key` and config set `cs`. + * + * When the configuration variable `key` is not found, returns 1 + * without touching `value`. + * + * The key will be parsed for validity with git_config_parse_key(), on + * error a negative value will be returned. See + * git_configset_get_knownkey_value_multi() for a version of this which + * BUG()s out on negative return values. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. + */ +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest); + +/** + * Like git_configset_get_value_multi(), but BUG()s out if the return + * value is < 0. Use it for keys known to pass git_config_parse_key(), + * i.e. those hardcoded in the code, and never user-provided keys. */ -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key); +int git_configset_get_knownkey_value_multi(struct config_set *cs, + const char *const key, + const struct string_list **dest); /** * Clears `config_set` structure, removes all saved variable-value pairs. @@ -495,8 +514,12 @@ struct repository; void repo_config(struct repository *repo, config_fn_t fn, void *data); int repo_config_get_value(struct repository *repo, const char *key, const char **value); -const struct string_list *repo_config_get_value_multi(struct repository *repo, - const char *key); +int repo_config_get_value_multi(struct repository *repo, + const char *key, + const struct string_list **dest); +int repo_config_get_knownkey_value_multi(struct repository *repo, + const char *const key, + const struct string_list **dest); int repo_config_get_string(struct repository *repo, const char *key, char **dest); int repo_config_get_string_tmp(struct repository *repo, @@ -543,10 +566,21 @@ int git_config_get_value(const char *key, const char **value); /** * Finds and returns the value list, sorted in order of increasing priority * for the configuration variable `key`. When the configuration variable - * `key` is not found, returns NULL. The caller should not free or modify - * the returned pointer, as it is owned by the cache. + * `key` is not found, returns 1 without touching `value`. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. + */ +int git_config_get_value_multi(const char *key, + const struct string_list **dest); + +/** + * A wrapper for git_config_get_value_multi() which does for it what + * git_configset_get_knownkey_value_multi() does for + * git_configset_get_value_multi(). */ -const struct string_list *git_config_get_value_multi(const char *key); +int git_config_get_knownkey_value_multi(const char *const key, + const struct string_list **dest); /** * Resets and invalidates the config cache. diff --git a/pack-bitmap.c b/pack-bitmap.c index 440407f1be7..0b4e73abbfa 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2301,7 +2301,12 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git) const struct string_list *bitmap_preferred_tips(struct repository *r) { - return repo_config_get_value_multi(r, "pack.preferbitmaptips"); + const struct string_list *dest; + + if (!repo_config_get_knownkey_value_multi(r, "pack.preferbitmaptips", + &dest)) + return dest; + return NULL; } int bitmap_is_preferred_refname(struct repository *r, const char *refname) diff --git a/submodule.c b/submodule.c index bf7a2c79183..e8c4362743d 100644 --- a/submodule.c +++ b/submodule.c @@ -274,8 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - sl = repo_config_get_value_multi(repo, "submodule.active"); - if (sl) { + if (!repo_config_get_knownkey_value_multi(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/helper/test-config.c b/t/helper/test-config.c index 4ba9eb65606..f0d476d2376 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -95,8 +95,7 @@ int cmd__config(int argc, const char **argv) goto exit1; } } else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) { - strptr = git_config_get_value_multi(argv[2]); - if (strptr) { + if (!git_config_get_knownkey_value_multi(argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) @@ -159,8 +158,7 @@ int cmd__config(int argc, const char **argv) goto exit2; } } - strptr = git_configset_get_value_multi(&cs, argv[2]); - if (strptr) { + if (!git_configset_get_knownkey_value_multi(&cs, argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) diff --git a/versioncmp.c b/versioncmp.c index 069ee94a4d7..9064478dc4a 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -160,10 +160,14 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { - const struct string_list *deprecated_prereleases; + const struct string_list *deprecated_prereleases = NULL; + initialized = 1; - prereleases = git_config_get_value_multi("versionsort.suffix"); - deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); + git_config_get_knownkey_value_multi("versionsort.suffix", + &prereleases); + git_config_get_value_multi("versionsort.prereleasesuffix", + &deprecated_prereleases); + if (prereleases) { if (deprecated_prereleases) warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); From patchwork Wed Oct 26 15:35:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020833 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 90BD7C38A2D for ; Wed, 26 Oct 2022 15:36:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234609AbiJZPgo (ORCPT ); Wed, 26 Oct 2022 11:36:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59110 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234574AbiJZPgk (ORCPT ); Wed, 26 Oct 2022 11:36:40 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DA4E412A371 for ; Wed, 26 Oct 2022 08:36:38 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id k8so18357979wrh.1 for ; Wed, 26 Oct 2022 08:36:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=jMe0F6isGj0OhIsQEnAue1kKs7CYrNzKni4aRD/aOt4=; b=l3qEVb/9xS91v59T8DMyzYxRbHorBz4ay9gBF9LJpNyW3mfVuZ9KUZ4zOdt7bJZEGq Bh3BYNUyykZBnEjIXSKOiAAM/57HjArBunKRJJi4nL01qM6BdDf6OlsiYEbO3S5U1riZ nS1AOKEhBhAbMuFMSJIFlUgwkfFMsFL2pSflrPX1FEfQy55kmsrhUMkMjPcVulXMSHe0 srT+/K1I1HbstL9FKD7Jdz/+BSD2OANR17n34w4HjP4668KKhLTHRYe7BuBqUEyJ5eaI PbNcWrmKPN+HlgsiADu2kDo0E81e0h6wFNLaj8deCNdgP6sjk/h/kSBvH2FzA9AWs2IS RQhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jMe0F6isGj0OhIsQEnAue1kKs7CYrNzKni4aRD/aOt4=; b=BSXVC3uNBDffM7PQstkIPUj2TNB7TvMajXau/yeOOGZ/bS514fNhVm9XsQDH61BeQI qc13KZboHya7a+1Mgr1r1utxV5kqp6KZAKh5z5ozknxxyYZwbqG09YukzT3qhMMK6aQi 1uSE/yPnT+h7oc5C4dWCrGOqCgDtd3BxW0g1yyZQIM0L5jRryZtlp6OQ45Hc7L0vanu0 eqOQumdXiWZXBjYIhLqkjDL/JEd0Uv9zrfIrjaeg5v54Nrr+59/wgIU6bnsWFsS35uiS hZMQ6YElE6HY6wR++GFqKp58M1e6vZbo5ZTKqhY/6RTv58xrMqNe58yB3/0/dT7hLWsz Ny1g== X-Gm-Message-State: ACrzQf1GcJNOOSDOL++K8DyPX9chikzRN5iomiRWzQDqC36tHItk2XYO bEOkmiMkJtglNybzSU1uyXgF2zSsELwStQ== X-Google-Smtp-Source: AMsMyM440MnWtXwp6WhwlzAM3Si82x4tpP3SRjYJP4hoShyRZOJBIUJyPYAFUdWirk453X3CMN418g== X-Received: by 2002:a05:6000:15cf:b0:230:ba81:cefc with SMTP id y15-20020a05600015cf00b00230ba81cefcmr29540257wry.544.1666798597102; Wed, 26 Oct 2022 08:36:37 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:36 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 02/10] for-each-repo: error on bad --config Date: Wed, 26 Oct 2022 17:35:15 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As noted in 6c62f015520 (for-each-repo: do nothing on empty config, 2021-01-08) this command wants to ignore a non-existing config key, but let's not conflate that with bad config. We could preserve the comment added in 6c62f015520, but now that we're directly using the documented repo_config_get_value_multi() value it's just narrating something that should be obvious from the API use, so let's drop it. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 15 +++++++-------- t/t0068-for-each-repo.sh | 6 ++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index b01721762ef..16e9a76d04a 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -28,7 +28,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) { static const char *config_key = NULL; int i, result = 0; - const struct string_list *values = NULL; + const struct string_list *values; + int err; const struct option options[] = { OPT_STRING(0, "config", &config_key, N_("config"), @@ -42,13 +43,11 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) if (!config_key) die(_("missing --config=")); - repo_config_get_value_multi(the_repository, config_key, &values); - - /* - * Do nothing on an empty list, which is equivalent to the case - * where the config variable does not exist at all. - */ - if (!values) + err = repo_config_get_value_multi(the_repository, config_key, &values); + if (err < 0) + usage_msg_optf(_("got bad config --config=%s"), + for_each_repo_usage, options, config_key); + else if (err) return 0; for (i = 0; !result && i < values->nr; i++) diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index 4675e852517..115221c9ca5 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -33,4 +33,10 @@ test_expect_success 'do nothing on empty config' ' git for-each-repo --config=bogus.config -- help --no-such-option ' +test_expect_success 'error on bad config keys' ' + test_expect_code 129 git for-each-repo --config=a && + test_expect_code 129 git for-each-repo --config=a.b. && + test_expect_code 129 git for-each-repo --config="'\''.b" +' + test_done From patchwork Wed Oct 26 15:35:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020834 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB101FA373E for ; Wed, 26 Oct 2022 15:36:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234611AbiJZPgr (ORCPT ); Wed, 26 Oct 2022 11:36:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59120 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234575AbiJZPgl (ORCPT ); Wed, 26 Oct 2022 11:36:41 -0400 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4214B120EDF for ; Wed, 26 Oct 2022 08:36:40 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id c3-20020a1c3503000000b003bd21e3dd7aso1823285wma.1 for ; Wed, 26 Oct 2022 08:36:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=hBxMwIrrD3RIY1ple08MOu9ais1lOY9tvPLWM8rylAQ=; b=iyOuT00psqdp3bm4dpvuh39X0PyXM0m794nZzCze07pjat/d0VeRVwDQMzwHk/C9+t Xzk27qOH65rx5KeFIEGDEKusfV4LLakMI80Ve6TEPqhQ6zKXheBAjPC+EWRUJzjZoDRD UFgUjsFD3LtNdAUPdRRpy0ayaGrWXf5APLV+i/90MKqX2J/4D8Ysu7ZjQTt/vmc4TAtA QBq2B/c/T/wK5atEhHxOFDJeHM09dSzv08tha8qlaROHvCbQnhz2mCCjXZ7jE6X9QhCy G/iQT/lIWCUFXHkIAEWOGHKeM5wumdiUMumq0sHOanPov1oZDbz1tpMrp8yN1FsNENg4 jcHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hBxMwIrrD3RIY1ple08MOu9ais1lOY9tvPLWM8rylAQ=; b=NorEccdpT00Jlv2+jCPDy5FsyuFAlk2SAO1hP6oheBv6pAMbYmKH4hzqQPtlvRkH/K 8Q4rE5e+njK8Mlbe8xDXnEBag3lVqI0LoCX3rS8OkRV4e9/qjsb9/Ak9a3M3B7faSd+C GX0rtO0CocGhG2aYM8DLCSAt4LaWLyq0e/QvOXvUD0u/G2nrE14/XS7udj8NydNA2Wys wuhsLg3zgqI8H/RWNpL36WF+qQlwB4rv1xN5pkJtjrmlertEzPnJ4tLE5C/KzarnjtUy qUurQ4Xb19OAPQWEvPKQYDTwxZSJIiVNWixdw7SsArDy+QnB+42z6cXpbHJ64/2BYisG cCVw== X-Gm-Message-State: ACrzQf35Jyk/Wu6OUn4jSjkoT0kravRQ2OqPNqrefksS63rW1M/4VYs9 sq3i+sRPRay9/zR0oonng1mez0UdTfPaAQ== X-Google-Smtp-Source: AMsMyM53YsZy9SHat40G3fbdYsGNEBtzh8iNzTQdoFb6Kj1LDgHtYLRiJQFnpWzBA1+Rzy8AqmrJYA== X-Received: by 2002:a05:600c:1d0f:b0:3c6:bfa9:9ef6 with SMTP id l15-20020a05600c1d0f00b003c6bfa99ef6mr2966060wms.136.1666798598445; Wed, 26 Oct 2022 08:36:38 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:37 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 03/10] config API: mark *_multi() with RESULT_MUST_BE_USED Date: Wed, 26 Oct 2022 17:35:16 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Use the RESULT_MUST_BE_USED attribute to assert that all users of the *_multi() API use the return values, in the preceding commit "for-each-repo" started using the return value meaningfully. This requires changing versioncmp() so that we use the "ret" versions of the return values, and don't implicitly rely on "deprecated_prereleases" being set to NULL if the key didn't exist. See 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01) for the introduction of RESULT_MUST_BE_USED. Signed-off-by: Ævar Arnfjörð Bjarmason --- config.h | 6 ++++++ versioncmp.c | 22 +++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/config.h b/config.h index c88619b7dcf..a5710c5856e 100644 --- a/config.h +++ b/config.h @@ -470,6 +470,7 @@ int git_configset_add_parameters(struct config_set *cs); * The caller should not free or modify the returned pointer, as it is * owned by the cache. */ +RESULT_MUST_BE_USED int git_configset_get_value_multi(struct config_set *cs, const char *key, const struct string_list **dest); @@ -478,6 +479,7 @@ int git_configset_get_value_multi(struct config_set *cs, const char *key, * value is < 0. Use it for keys known to pass git_config_parse_key(), * i.e. those hardcoded in the code, and never user-provided keys. */ +RESULT_MUST_BE_USED int git_configset_get_knownkey_value_multi(struct config_set *cs, const char *const key, const struct string_list **dest); @@ -514,9 +516,11 @@ struct repository; void repo_config(struct repository *repo, config_fn_t fn, void *data); int repo_config_get_value(struct repository *repo, const char *key, const char **value); +RESULT_MUST_BE_USED int repo_config_get_value_multi(struct repository *repo, const char *key, const struct string_list **dest); +RESULT_MUST_BE_USED int repo_config_get_knownkey_value_multi(struct repository *repo, const char *const key, const struct string_list **dest); @@ -571,6 +575,7 @@ int git_config_get_value(const char *key, const char **value); * The caller should not free or modify the returned pointer, as it is * owned by the cache. */ +RESULT_MUST_BE_USED int git_config_get_value_multi(const char *key, const struct string_list **dest); @@ -579,6 +584,7 @@ int git_config_get_value_multi(const char *key, * git_configset_get_knownkey_value_multi() does for * git_configset_get_value_multi(). */ +RESULT_MUST_BE_USED int git_config_get_knownkey_value_multi(const char *const key, const struct string_list **dest); diff --git a/versioncmp.c b/versioncmp.c index 9064478dc4a..effe1a6a6be 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -160,19 +160,23 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { - const struct string_list *deprecated_prereleases = NULL; + const struct string_list *deprecated_prereleases; + int prereleases_ret, deprecated_prereleases_ret; initialized = 1; - git_config_get_knownkey_value_multi("versionsort.suffix", - &prereleases); - git_config_get_value_multi("versionsort.prereleasesuffix", - &deprecated_prereleases); - - if (prereleases) { - if (deprecated_prereleases) + prereleases_ret = + git_config_get_knownkey_value_multi("versionsort.suffix", + &prereleases); + deprecated_prereleases_ret = + git_config_get_knownkey_value_multi("versionsort.prereleasesuffix", + &deprecated_prereleases); + + if (!prereleases_ret) { + if (!deprecated_prereleases_ret) warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); - } else + } else if (!deprecated_prereleases_ret) { prereleases = deprecated_prereleases; + } } if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1, &diff)) From patchwork Wed Oct 26 15:35:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020835 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75ADEC433FE for ; Wed, 26 Oct 2022 15:36:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234615AbiJZPgu (ORCPT ); Wed, 26 Oct 2022 11:36:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59138 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234598AbiJZPgm (ORCPT ); Wed, 26 Oct 2022 11:36:42 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 787C266851 for ; Wed, 26 Oct 2022 08:36:41 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id i5-20020a1c3b05000000b003cf47dcd316so1813969wma.4 for ; Wed, 26 Oct 2022 08:36:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=56jtj266cXjpOowOwFVePvzRhJJnqtzHc0RD46ZRiTs=; b=eixtWnVQVIq4K4r2GDHGRNrM+TIZ9May+IwYo0coReEqwCNzz7uavdPBdpncleh8Vu Rj0emh7zgi6MkwEnM+SKlI/lQu6e8avV9YJ2hx6DP+ExKdHvambJknk27AV8l3zR2bTu 0XcRf1Cp7MO1JvXFpM7tFEfiw2I5xKeRG8G8LW18PsIDovj4PpmRGnxBiO3djRRYYVmm z3y1xCedlMfdapcZxG9m3WvF+p2o1+cBs8UB7MoG6NIW3NLC9UDAJthqWryq8O/YBzLv kAXqNHrv5RoOETQFlV6kl5vBTsCTzr2fCvyi8BmO6Zz8V3y5P53rt88f93vNkx5pYkIO 2b0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=56jtj266cXjpOowOwFVePvzRhJJnqtzHc0RD46ZRiTs=; b=Jk588skfkgbGncxVmbrCd37ioeYUyzMMwOckFC7I6y+InYINmvbzaZHUElgPfdI582 7tqmqEWL2sEfW/TVJQsx4j1bQ+BkZRle2N+4SEl3xGt0N7TLT06JT6w5xtsIZzy8bc9y ULwf/2L50u1alBllSSx3ct5GwCyvmZnrDubU1eIIiQaLOwQv449C7CSjClz5+qIqgyUc iL8pIwEFdmfJ+4GRa5pH7mF5153mtnqJKnaPvIshR1mghiJmgR3q4LL7FvJcAGAVEvIl q2SbWRDWz7noPHQot2rmOz/SiX4DqBda33M/MUfqx4ziBywRzDgUe4BPE/6CnhcqIvQ9 U13g== X-Gm-Message-State: ACrzQf3Y/iQF/5wbsh4U5zNjGASoyWFtCKY322UhcUe8CfaOiUOjkhYY YHMrs5w0kg1CEudr2LXItgHiRs0N97Xjtw== X-Google-Smtp-Source: AMsMyM6MLQiw3F0QG4E9WfIcqVAiNwQDTEt7zNut5fJdRhBxf8NFccrPz/lHsqaMt40l2UoVNQRw4w== X-Received: by 2002:a05:600c:4113:b0:3c6:f5d7:aa8d with SMTP id j19-20020a05600c411300b003c6f5d7aa8dmr2968825wmi.167.1666798599643; Wed, 26 Oct 2022 08:36:39 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:38 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 04/10] string-list API: mark "struct_string_list" to "for_each_string_list" const Date: Wed, 26 Oct 2022 17:35:17 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a "const" to the "struct string_list *" passed to for_each_string_list(). This is arguably abuse of the type system, as the "string_list_each_func_t fn" take a "struct string_list_item *", i.e. not one with a "const", and those functions *can* modify those items. But as we'll see in a subsequent commit we have other such iteration functions that could benefit from a "const", i.e. to declare that we're not altering the list itself, even though we might be calling functions that alter its values. Signed-off-by: Ævar Arnfjörð Bjarmason --- string-list.c | 2 +- string-list.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/string-list.c b/string-list.c index 549fc416d68..d8957466d25 100644 --- a/string-list.c +++ b/string-list.c @@ -129,7 +129,7 @@ void string_list_remove_duplicates(struct string_list *list, int free_util) } } -int for_each_string_list(struct string_list *list, +int for_each_string_list(const struct string_list *list, string_list_each_func_t fn, void *cb_data) { int i, ret = 0; diff --git a/string-list.h b/string-list.h index c7b0d5d0008..7153cb79154 100644 --- a/string-list.h +++ b/string-list.h @@ -138,7 +138,7 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c * Apply `func` to each item. If `func` returns nonzero, the * iteration aborts and the return value is propagated. */ -int for_each_string_list(struct string_list *list, +int for_each_string_list(const struct string_list *list, string_list_each_func_t func, void *cb_data); /** From patchwork Wed Oct 26 15:35:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020836 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0E3D5C38A2D for ; Wed, 26 Oct 2022 15:36:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234617AbiJZPgz (ORCPT ); Wed, 26 Oct 2022 11:36:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59142 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234599AbiJZPgm (ORCPT ); Wed, 26 Oct 2022 11:36:42 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DA63A120EEE for ; Wed, 26 Oct 2022 08:36:41 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id k8so18358352wrh.1 for ; Wed, 26 Oct 2022 08:36:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=11L7QHXASbcJQWfdTU2dm4xZChgsJ6GjlkxUUJWQhmo=; b=XDh4WWExM1FjsdeJeNEY0sFwhC0zvkzMdAz4fCnmEeOH2dyjnlecN7Oc45/CevlnLy CccBGmAH05GGc4BE9rZ/pu7e0z+nrLZXtlJb85GkKQby/OYxSr2OCBLxHMr1JP3Bhuzy jDJqHME1kAUIeh0vgzGmN07bEbVRlgvBBHa5JDtzcU04y27X5WSvW6e077FQSsrV+xdX pc3m20pIowSca8Y9+yePCypY/ASA+S+ofXFHzppCs5zdWsv+0lUTACanRvsRty6zgLAX ET2nY2+d4J7Sef0JB/T3IrxsypqNifeJX7WxhDp9SX0IBb40Ttyyi6aWUVS8JkwF4k8t 715g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=11L7QHXASbcJQWfdTU2dm4xZChgsJ6GjlkxUUJWQhmo=; b=ZZCNijRHIDXH0CkQV2u21dKUOzKAE+hcgJLQUKV/cmN8DAAZH+u/YqMsUmqMXzL3WL jc+ipTZ+P9Uk9jr++8OpMBHiMHHK/kKf2a0UlxyRSKCwO8uis3kzvv2PDB/MOVNV0kSk SN7epuOiRNM2yB9jmo4mPR8tZZzzdrKvFZn0Wbwprr5XEt7+QNb4Gm1frQ+OVh7a2fnX ZGewbRzCGDveHm04/mOsMkz9sQ1nJrn2ZnVDkb2f1icZi1GOCo7k3tBrBWrdO6WO/R2A lmHkWLrmyKXFIbM5lq8rDxzS6wgVDQgdVBFRSJbLOG4vNhGZmnKdHh2d89r5hMU8aug8 vOFw== X-Gm-Message-State: ACrzQf24S/Hf899kLif8cS82KqGhBBv6o8gHQkXY5mTZZjeJxhxHbmzW MKrCGQTvOtU5NCULRcfwPIpVNwRQEg4q+w== X-Google-Smtp-Source: AMsMyM5OgI+b4c+1uonMO9nnxlNAOY7cxrbIOjFMOasIU1xcfcBrqVLbBwp0+4y6iD7d5qwNYxwx+Q== X-Received: by 2002:a05:6000:2c1:b0:22e:7b7e:de28 with SMTP id o1-20020a05600002c100b0022e7b7ede28mr28408316wry.123.1666798601052; Wed, 26 Oct 2022 08:36:41 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:40 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 05/10] string-list API: make has_string() and list_lookup() "const" Date: Wed, 26 Oct 2022 17:35:18 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Ever since these were added in the "path_list" predecessor of this API in 6d297f81373 (Status update on merge-recursive in C, 2006-07-08) they haven't been "const", but as the compiler validates for us adding that attribute to them is correct. Note that they will return a non-const "struct string_list_item *", but the "struct string_list *" itself that's passed in can be marked "const". Signed-off-by: Ævar Arnfjörð Bjarmason --- string-list.c | 4 ++-- string-list.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/string-list.c b/string-list.c index d8957466d25..d97a8f61c02 100644 --- a/string-list.c +++ b/string-list.c @@ -245,7 +245,7 @@ void string_list_sort(struct string_list *list) QSORT_S(list->items, list->nr, cmp_items, &sort_ctx); } -struct string_list_item *unsorted_string_list_lookup(struct string_list *list, +struct string_list_item *unsorted_string_list_lookup(const struct string_list *list, const char *string) { struct string_list_item *item; @@ -257,7 +257,7 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list, return NULL; } -int unsorted_string_list_has_string(struct string_list *list, +int unsorted_string_list_has_string(const struct string_list *list, const char *string) { return unsorted_string_list_lookup(list, string) != NULL; diff --git a/string-list.h b/string-list.h index 7153cb79154..3589afee2ee 100644 --- a/string-list.h +++ b/string-list.h @@ -227,13 +227,13 @@ void string_list_sort(struct string_list *list); * Like `string_list_has_string()` but for unsorted lists. Linear in * size of the list. */ -int unsorted_string_list_has_string(struct string_list *list, const char *string); +int unsorted_string_list_has_string(const struct string_list *list, const char *string); /** * Like `string_list_lookup()` but for unsorted lists. Linear in size * of the list. */ -struct string_list_item *unsorted_string_list_lookup(struct string_list *list, +struct string_list_item *unsorted_string_list_lookup(const struct string_list *list, const char *string); /** * Remove an item from a string_list. The `string` pointer of the From patchwork Wed Oct 26 15:35:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020837 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5E88C38A2D for ; Wed, 26 Oct 2022 15:37:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234635AbiJZPhN (ORCPT ); Wed, 26 Oct 2022 11:37:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59210 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234610AbiJZPgp (ORCPT ); Wed, 26 Oct 2022 11:36:45 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74F70130D5C for ; Wed, 26 Oct 2022 08:36:44 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id bk15so26992091wrb.13 for ; Wed, 26 Oct 2022 08:36:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=9B5zI19cDlP59KQZBSGfNuJCK9G1ziDbj8x+wqZkgYw=; b=mHGUcAdxECQ79oektdh+8CEtKCXvnLJRmwdD9Lm5RSn51N+D++DuW7NdbKFO39JddX E7/B/Mm21g6tWiaNmS0O2muhoDvyXFf+g1TdckpUswoq9/IANp0RPDPr3cm/G0Tp+w4Z DRWDQKvjSXBHUxeMnMgJaG70IEJq0K8I93mUz5eHGqEHnGNz9UcNV/yAYzaeWp2lU+QN MMqFFGdoDa/sC1iD7mZI9nf8gJqZYZOdrGHxY2sma1ejlmFLYAPlF7XZKZ+l4P0OIabk EQfMJn7DdmpDO9HBFcY1sU3ah69qyKkl5HyRlQW8rxdhVNHBsZIvP5/GPmRQ41k0QKm6 YFPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9B5zI19cDlP59KQZBSGfNuJCK9G1ziDbj8x+wqZkgYw=; b=5miymsoS4wQ/O4cLA3dO5rxqR/m46lDNFpPmQyMqAbpS5F7YZQm2m6zPRgZ5a4sFX4 EBS0y2i+5bX005FnzB+l+prprxfOr4OdlXDU6ldrq7BQVTx2W91IspfrIIsAwpQt6MPL wk2w7mikyrtiP2nfXHEQTIuedjAUM9YZ5ysowaT1HFGITdI+ganPQ9K8YWo3Eo5QqWWU tA/1i7HuOM75j/yl4JNue+kzL2uBRnK7HLkGUUbN1+wJqx+qkxj6UL/7obzW/7ZjHH1T yFWFRO7xHthlDVBR/e1VdTeCraigHis50gv9oeCfC+bFkuM8ptCm8cUD/aK6mEY0UgH2 VgPw== X-Gm-Message-State: ACrzQf11Fiw0lJFJIujVhC1eFY2lZpLl6b6iMbn7XJrr02WWRaK85x6B olSRg0a6KTTZd4C9srZBttK0P3Mje/XFwQ== X-Google-Smtp-Source: AMsMyM5MfO5D7WW57/NvnJRm0Vwzy46p461OG/ViyJyPXZDLaUWqZIdIEIuGQCn8mOkoX3nLkk2qUw== X-Received: by 2002:a05:6000:2ad:b0:231:48fb:3a64 with SMTP id l13-20020a05600002ad00b0023148fb3a64mr28517351wry.184.1666798602676; Wed, 26 Oct 2022 08:36:42 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:41 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 06/10] builtin/gc.c: use "unsorted_string_list_has_string()" where appropriate Date: Wed, 26 Oct 2022 17:35:19 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Refactor a "do I have an element like this?" pattern added in [1] and [2] to use unsorted_string_list_has_string() instead of a for_each_string_list_item() loop. A preceding commit added a "const" to the "struct string_list *" argument of unsorted_string_list_has_string(), it'll thus play nicely with git_config_get_const_value_multi() without needing a cast here. 1. 1ebe6b02970 (maintenance: add 'unregister --force', 2022-09-27) 2. 50a044f1e40 (gc: replace config subprocesses with API calls, 2022-09-27) Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 04c48638ef4..f435eda2e73 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1467,7 +1467,6 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) const char *key = "maintenance.repo"; char *config_value; char *maintpath = get_maintpath(); - struct string_list_item *item; const struct string_list *list; argc = parse_options(argc, argv, prefix, options, @@ -1485,14 +1484,8 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) else git_config_set("maintenance.strategy", "incremental"); - if (!git_config_get_knownkey_value_multi(key, &list)) { - for_each_string_list_item(item, list) { - if (!strcmp(maintpath, item->string)) { - found = 1; - break; - } - } - } + if (!git_config_get_knownkey_value_multi(key, &list)) + found = unsorted_string_list_has_string(list, maintpath); if (!found) { int rc; @@ -1532,7 +1525,6 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi const char *key = "maintenance.repo"; char *maintpath = get_maintpath(); int found = 0; - struct string_list_item *item; const struct string_list *list; argc = parse_options(argc, argv, prefix, options, @@ -1541,14 +1533,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi usage_with_options(builtin_maintenance_unregister_usage, options); - if (!git_config_get_knownkey_value_multi(key, &list)) { - for_each_string_list_item(item, list) { - if (!strcmp(maintpath, item->string)) { - found = 1; - break; - } - } - } + if (!git_config_get_knownkey_value_multi(key, &list)) + found = unsorted_string_list_has_string(list, maintpath); if (found) { int rc; From patchwork Wed Oct 26 15:35:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020839 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D1CD5C433FE for ; Wed, 26 Oct 2022 15:37:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234601AbiJZPhT (ORCPT ); Wed, 26 Oct 2022 11:37:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234619AbiJZPhL (ORCPT ); Wed, 26 Oct 2022 11:37:11 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2BCF132DC7 for ; Wed, 26 Oct 2022 08:36:45 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id g12so14213646wrs.10 for ; Wed, 26 Oct 2022 08:36:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=/YkOAAVekhipes04yfRYM4Wej+5sf5u/ItiK+Lwg/1U=; b=YsElhwWzQ5GwXf024qWOs0BUwtEAi3YOWDlkM6BMaxz0zMudTQUzlQ7owdFJZm7D0S NSqxSKkvLJFBQrWUYXWyd8WHjWO6FdEXF+oX7zkIljeGArYgeW9B/3NOgB+1ttODo7zJ lr+DeWIY9mPkxX5zBYHObz8oBFG5PE9yCsL+epc24IW0/e7qGrhUMPElyJjdaEiqwYeK 2cHbVgMTDMFWO6vm5mQ7ShFmnyPudf01aXD19jSWyqqrJq0yOs3w5pS/ocu14zF3MgYj p+DKFdZ3lvO18gDyjLlCOZlQxLf5Sx1MhcSHv764/rcqbDQoHL/KjNj7Bm5DP8FZ7mQO h4GA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/YkOAAVekhipes04yfRYM4Wej+5sf5u/ItiK+Lwg/1U=; b=CvPevSsilTkwRAvYq/u/jDOx47leYsy3brVp/p7CxYkx+lGr/uRzipU3c1zcIfoEnu pqFZjy1U1JR04zSyrtppJlU2HE0rQv42eA244BbZ9+WMh96KkbfTEyNjcVmPnaW/I8vE CYrLbBpNLfqsNzW4QDYLESpFNtznRaJ5AcjPTmvUtq9jZtT+NXM95cLth6X+W3Hp27N8 rQhDbq2kdNYKX4fQpHrXoeeDD9MyirM14TS/Op2mePxbm0gwvm3GcQtQnuX4Rx8if2cZ HuIHiFBnTt4zxS54gx2sCuyk4KobcVYUhHFToongJyoxImoBbGm/Fn24oRJKYBqjFuT0 b52w== X-Gm-Message-State: ACrzQf12QQcNHEJHdEGAey9SeWVL1PXBUOAVJ3jc+Nw6BZcStDuFaUTu 5qGSgWddd9F8U5b3honTcSfwL2hvlFE+wg== X-Google-Smtp-Source: AMsMyM5jzgrDKRAvb2jhepov6bKjHv9wSzmY7o9nP9K5WsT7Vkj9X01baJe6TWEFCyevgcksPi/02A== X-Received: by 2002:adf:e583:0:b0:236:6280:57c9 with SMTP id l3-20020adfe583000000b00236628057c9mr14872986wrm.262.1666798603728; Wed, 26 Oct 2022 08:36:43 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:43 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 07/10] config API: add and use "lookup_value" functions Date: Wed, 26 Oct 2022 17:35:20 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Change various users of the config API who only wanted to ask if a configuration key existed to use a new *_config*_lookup_value() family of functions. Unlike the existing API functions in the API this one doesn't take a "dest" argument. Some of these were using either git_config_get_string() or git_config_get_string_tmp(), see fe4c750fb13 (submodule--helper: fix a configure_added_submodule() leak, 2022-09-01) for a recent example. We can now use a helper function that doesn't require a throwaway variable. We could have changed git_configset_get_value_multi() to accept a "NULL" as a "dest" for all callers, but let's avoid changing the behavior of existing API users. The new "lookup" API and the older API call our static "git_configset_get_value_multi_1()" helper with a new "read_only" argument instead. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 5 +---- builtin/submodule--helper.c | 9 +++------ builtin/worktree.c | 3 +-- config.c | 25 +++++++++++++++++++++---- config.h | 12 ++++++++++++ 5 files changed, 38 insertions(+), 16 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index f435eda2e73..3e94fa5e20f 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1465,7 +1465,6 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) }; int found = 0; const char *key = "maintenance.repo"; - char *config_value; char *maintpath = get_maintpath(); const struct string_list *list; @@ -1479,9 +1478,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) git_config_set("maintenance.auto", "false"); /* Set maintenance strategy, if unset */ - if (!git_config_get_string("maintenance.strategy", &config_value)) - free(config_value); - else + if (git_config_lookup_value("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); if (!git_config_get_knownkey_value_multi(key, &list)) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1f8fe6a8e0d..b758255f816 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -541,7 +541,6 @@ static int module_init(int argc, const char **argv, const char *prefix) NULL }; int ret = 1; - const struct string_list *values; argc = parse_options(argc, argv, prefix, module_init_options, git_submodule_helper_usage, 0); @@ -553,7 +552,7 @@ static int module_init(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && !git_config_get_value_multi("submodule.active", &values)) + if (!argc && !git_config_lookup_value("submodule.active")) module_list_active(&list); info.prefix = prefix; @@ -2709,7 +2708,6 @@ static int module_update(int argc, const char **argv, const char *prefix) if (opt.init) { struct module_list list = MODULE_LIST_INIT; struct init_cb info = INIT_CB_INIT; - const struct string_list *values; if (module_list_compute(argc, argv, opt.prefix, &pathspec2, &list) < 0) { @@ -2722,7 +2720,7 @@ static int module_update(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && !git_config_get_value_multi("submodule.active", &values)) + if (!argc && !git_config_lookup_value("submodule.active")) module_list_active(&list); info.prefix = opt.prefix; @@ -3166,7 +3164,6 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con static void configure_added_submodule(struct add_data *add_data) { char *key; - const char *val; struct child_process add_submod = CHILD_PROCESS_INIT; struct child_process add_gitmodules = CHILD_PROCESS_INIT; @@ -3211,7 +3208,7 @@ static void configure_added_submodule(struct add_data *add_data) * is_submodule_active(), since that function needs to find * out the value of "submodule.active" again anyway. */ - if (!git_config_get_string_tmp("submodule.active", &val)) { + if (!git_config_lookup_value("submodule.active")) { /* * If the submodule being added isn't already covered by the * current configured pathspec, set the submodule's active flag diff --git a/builtin/worktree.c b/builtin/worktree.c index c6710b25520..5ab16631dbc 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -260,7 +260,6 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir) if (file_exists(from_file)) { struct config_set cs = { { 0 } }; - const char *core_worktree; int bare; if (safe_create_leading_directories(to_file) || @@ -279,7 +278,7 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir) to_file, "core.bare", NULL, "true", 0)) error(_("failed to unset '%s' in '%s'"), "core.bare", to_file); - if (!git_configset_get_value(&cs, "core.worktree", &core_worktree) && + if (!git_configset_lookup_value(&cs, "core.worktree") && git_config_set_in_file_gently(to_file, "core.worktree", NULL)) error(_("failed to unset '%s' in '%s'"), diff --git a/config.c b/config.c index 2100b29b689..5cd130ddbb9 100644 --- a/config.c +++ b/config.c @@ -2428,7 +2428,7 @@ int git_configset_get_value(struct config_set *cs, const char *key, const char * static int git_configset_get_value_multi_1(struct config_set *cs, const char *key, const struct string_list **dest, - int knownkey) + int read_only, int knownkey) { struct config_set_element *e; int ret; @@ -2440,7 +2440,8 @@ static int git_configset_get_value_multi_1(struct config_set *cs, const char *ke return ret; else if (!e) return 1; - *dest = &e->value_list; + if (!read_only) + *dest = &e->value_list; return 0; } @@ -2448,14 +2449,19 @@ static int git_configset_get_value_multi_1(struct config_set *cs, const char *ke int git_configset_get_value_multi(struct config_set *cs, const char *key, const struct string_list **dest) { - return git_configset_get_value_multi_1(cs, key, dest, 0); + return git_configset_get_value_multi_1(cs, key, dest, 0, 0); } int git_configset_get_knownkey_value_multi(struct config_set *cs, const char *const key, const struct string_list **dest) { - return git_configset_get_value_multi_1(cs, key, dest, 1); + return git_configset_get_value_multi_1(cs, key, dest, 0, 1); +} + +int git_configset_lookup_value(struct config_set *cs, const char *key) +{ + return git_configset_get_value_multi_1(cs, key, NULL, 1, 0); } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -2594,6 +2600,12 @@ void repo_config(struct repository *repo, config_fn_t fn, void *data) configset_iter(repo->config, fn, data); } +int repo_config_lookup_value(struct repository *repo, const char *key) +{ + git_config_check_init(repo); + return git_configset_get_value_multi_1(repo->config, key, NULL, 1, 0); +} + int repo_config_get_value(struct repository *repo, const char *key, const char **value) { @@ -2726,6 +2738,11 @@ void git_config_clear(void) repo_config_clear(the_repository); } +int git_config_lookup_value(const char *key) +{ + return repo_config_lookup_value(the_repository, key); +} + int git_config_get_value(const char *key, const char **value) { return repo_config_get_value(the_repository, key, value); diff --git a/config.h b/config.h index a5710c5856e..cf1ae7862a8 100644 --- a/config.h +++ b/config.h @@ -502,6 +502,8 @@ void git_configset_clear(struct config_set *cs); * is owned by the cache. */ int git_configset_get_value(struct config_set *cs, const char *key, const char **dest); +RESULT_MUST_BE_USED +int git_configset_lookup_value(struct config_set *cs, const char *key); int git_configset_get_string(struct config_set *cs, const char *key, char **dest); int git_configset_get_int(struct config_set *cs, const char *key, int *dest); @@ -524,6 +526,8 @@ RESULT_MUST_BE_USED int repo_config_get_knownkey_value_multi(struct repository *repo, const char *const key, const struct string_list **dest); +RESULT_MUST_BE_USED +int repo_config_lookup_value(struct repository *repo, const char *key); int repo_config_get_string(struct repository *repo, const char *key, char **dest); int repo_config_get_string_tmp(struct repository *repo, @@ -588,6 +592,14 @@ RESULT_MUST_BE_USED int git_config_get_knownkey_value_multi(const char *const key, const struct string_list **dest); +/** + * The same as git_config_value(), except without the extra work to + * return the value to the user, used to check if a value for a key + * exists. + */ +RESULT_MUST_BE_USED +int git_config_lookup_value(const char *key); + /** * Resets and invalidates the config cache. */ From patchwork Wed Oct 26 15:35:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020838 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B5ADC38A2D for ; Wed, 26 Oct 2022 15:37:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234673AbiJZPhQ (ORCPT ); Wed, 26 Oct 2022 11:37:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60052 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234612AbiJZPhM (ORCPT ); Wed, 26 Oct 2022 11:37:12 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 46D90132DFF for ; Wed, 26 Oct 2022 08:36:48 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id y16so16547732wrt.12 for ; Wed, 26 Oct 2022 08:36:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=0ua905TbBf8B++yzE8Tff48m+J1eagMgWNViFnhjuZM=; b=IKPvWaAgkWLnJiif4uYUpkJBDWC6QQPV9PzqNfDCj2cgpEfKwCJrRUezYtexuvxuzE Jl+J3v6+rblP9WFquvONz0Gi6jiPtPUaXp3G2S8cuStmoNwJKQSMNSXUSyAko7YykXKK OiMFCvwwdubcVZKa4x+zgpUeqfCylUlXlcHTXCdiWrbrf7+8/IeVlgNxUAB8f+IgyEbu R8Mz0eR3phOA/uzBMDhrgZ+XBK5zYjOooE0X14h279Q6KUlHs2Mn+nF/0kZPHG+sR9IN KuaPdaNvigS2dOw0+YWVfwp5fKXQ+2zU77hls50G+1mgySoPYcJ2k8HdJycCGeWtw4uM 30yQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0ua905TbBf8B++yzE8Tff48m+J1eagMgWNViFnhjuZM=; b=Aj+dhZSu9Y0JNGQOCvH+mBFvxiC4C7Vnq49/1DAIqu4qcuPwCfPdw/5mQkFkpJZ6Po 0Nh2qUfdy4qbka2HgCOAsQ2OnQf+zY4i6e3AfTv/0VCt5fL+IPzzKlVJiuwhKST+N61k 6VfNM5VzUUbCHBvADwNIExxme2BzPt4Du7YQ64pRiBWqUZVebB0L1Jl33qZNVOYlZw7B eVBlHg+YRN1RehAveq2N1NzF8aNKn0TmLpadKMXWn8Q1Xkko/AVj3d37tuUQK7cnMPpZ wU7MxO8LvtQTx8hSPkDGtoy0ilqLjNr3m9OOG54GVrZAz8O9FWiMJvizS/CLvURFMPVM Z6mQ== X-Gm-Message-State: ACrzQf3EI4dOQqAeJ+08d/p6MUiCWHy01dlpVNQJHf+3/OeJc1yG01++ e6lpTnJO3QN1NQfTyAHQIc5TrHUoys61cQ== X-Google-Smtp-Source: AMsMyM4QPoL8nBthiqaOT2EGx0vCb+qaIHkdeAFbnXQtOlRVpmcEzk+A4zD/9PvbOxLbaqxYYU6L4A== X-Received: by 2002:a5d:598d:0:b0:236:8ef4:7324 with SMTP id n13-20020a5d598d000000b002368ef47324mr1936683wri.84.1666798606542; Wed, 26 Oct 2022 08:36:46 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:44 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 08/10] config tests: add "NULL" tests for *_get_value_multi() Date: Wed, 26 Oct 2022 17:35:21 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A less well known edge case in the config format is that keys can be value-less, a shorthand syntax for "true" boolean keys. I.e. these two are equivalent as far as "--type=bool" is concerned: [a]key [a]key = true But as far as our parser is concerned the values for these two are NULL, and "true". I.e. for a sequence like: [a]key=x [a]key [a]key=y We get a "struct string_list" with "string" members with ".string" values of: { "x", NULL, "y" } This behavior goes back to the initial implementation of git_config_bool() in 17712991a59 (Add ".git/config" file parser, 2005-10-10). When the "t/t1308-config-set.sh" tests were added in [1] only one of the three "(NULL)" lines in "t/helper/test-config.c" had any test coverage. This change adds tests that stress the remaining two. 1. 4c715ebb96a (test-config: add tests for the config_set API, 2014-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t1308-config-set.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index b38e158d3b2..561e82f1808 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -146,6 +146,36 @@ test_expect_success 'find multiple values' ' check_config get_value_multi case.baz sam bat hask ' +test_expect_success 'emit multi values from configset with NULL entry' ' + test_when_finished "rm -f my.config" && + cat >my.config <<-\EOF && + [a]key=x + [a]key + [a]key=y + EOF + cat >expect <<-\EOF && + x + (NULL) + y + EOF + test-tool config configset_get_value_multi a.key my.config >actual && + test_cmp expect actual +' + +test_expect_success 'multi values from configset with a last NULL entry' ' + test_when_finished "rm -f my.config" && + cat >my.config <<-\EOF && + [a]key=x + [a]key=y + [a]key + EOF + cat >expect <<-\EOF && + (NULL) + EOF + test-tool config configset_get_value a.key my.config >actual && + test_cmp expect actual +' + test_expect_success 'find value from a configset' ' cat >config2 <<-\EOF && [case] From patchwork Wed Oct 26 15:35:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020841 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DCC7C38A2D for ; Wed, 26 Oct 2022 15:37:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234390AbiJZPhY (ORCPT ); Wed, 26 Oct 2022 11:37:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59286 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234599AbiJZPhN (ORCPT ); Wed, 26 Oct 2022 11:37:13 -0400 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15B10132EB7 for ; Wed, 26 Oct 2022 08:36:49 -0700 (PDT) Received: by mail-wr1-x432.google.com with SMTP id bp11so26453547wrb.9 for ; Wed, 26 Oct 2022 08:36:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=aoYQgBb7Hvs24wGCL0DyTLCeg3CW9CIYsQYGy2GbvNw=; b=Vu02bhzVYar/osKXeF/LkFRBUZd6MQAZAFMpMwvRZQhDIP9OH4+Sl7N4VEinWoom+V 3wVllSuTKOr7Bfegtmksu7FgBUPzwk8OxFR45QNbww0TPVjeXNJGLo4cKUxD8uPE4FeY 8BGOJXM+ORh5CmTUdnZQ/bJz0AF3RElll7qPXCFhC9F0ETubBWYn0bzUy5bhf5Ljmyq+ 6dw668DktCw+YUmI+iuPHclk1+YxLT8ajqEfIQse12uASgvZSn+3rC4VpsvphF4mc0UG 2VBGpNRCtDXd5pII7wuPveAxUz7bjp54/hg7BxEnwv6SZBwAfSzBoT+SarSgEb7xropY 0gLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=aoYQgBb7Hvs24wGCL0DyTLCeg3CW9CIYsQYGy2GbvNw=; b=w5qCqZSvhVdpQbDFJ+A0eWOv7IYYjfkZC2FwK98oGoO6AsAXGXLS2w+WcV9C0AxRbG LTSNdA3d1qcukQyM3i4nxi/GSpCqS/gDg9K/3xQEJsXYuDfpHn8Q++YUEF6vQExsfder AN4fregf3zFQDaqS8scobWlL3X15Z/pL7gncRjyQ4cNg9kS5Yc8woQY/IJHaT0GjunFx qTrK4+XxgpWZG9/BBfY9dsTzsFzFT+ZR+xAejmu0IvPNEnE8YebIzgaji3sUGq612f42 Jk3BLrqxAIQj0WITGNQ7RfBgZzEXNO2A7cvgLB9n/m8Lqoit1jgeVNKMD87bEEo3vhwZ xa6Q== X-Gm-Message-State: ACrzQf2mgSNBHVVcnBcDize1T4Fbsld/wgWu/IfshjTC8VdHfOaanewg WZXPSHgldXnsfybUbSEXzaGETJojtESYKA== X-Google-Smtp-Source: AMsMyM5LcDzU1bbD2U4XEvC/J0xOtZF1wPgasEl9vpAzLfZzQhTLjjOoI2D3GZNjTuWOQ9IkNG+g1A== X-Received: by 2002:a05:6000:184d:b0:22f:4ef4:47a7 with SMTP id c13-20020a056000184d00b0022f4ef447a7mr29230436wri.563.1666798607944; Wed, 26 Oct 2022 08:36:47 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:47 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 09/10] config API: add "string" version of *_value_multi(), fix segfaults Date: Wed, 26 Oct 2022 17:35:22 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix numerous and mostly long-standing segfaults in consumers of the *_config_*value_multi() API. As discussed in the preceding commit an empty key in the config syntax yields a "NULL" string, which these users would give to strcmp() (or similar), resulting in segfaults. As this change shows, these non-test users of the *_config_*value_multi() API didn't really want such an an unsafe and low-level API, let's give them something with the safety of git_config_get_string() instead. This fix is similar to what the *_string() functions and others acquired in[1] and [2]. Namely introducing and using a safer *_config_*value_multi_string() variant of the low-level *_config_*value_multi_string() function. This fixes segfaults in code introduced in: - d811c8e17c6 (versionsort: support reorder prerelease suffixes, 2015-02-26) - c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) - a086f921a72 (submodule: decouple url and submodule interest, 2017-03-17) - a6be5e6764a (log: add log.excludeDecoration config option, 2020-04-16) - 92156291ca8 (log: add default decoration filter, 2022-08-05) - 50a044f1e40 (gc: replace config subprocesses with API calls, 2022-09-27) There are now two remaining user of the low-level API, one is the "t/helper/test-config.c" code added in [3]. The other we'll address in a subsequent commit. As seen in the preceding commit we need to give the "t/helper/test-config.c" caller these "NULL" entries. We thus cannot alter the underlying git_configset_get_value_multi_1() function itself to make it "safe". Such a thing would also be undesirable, as casting or forbidding NULL values might only be one potential use-case of the underlying function. It's better to have a "raw" low-level function, and corresponding wrapper functions that coerce its values. The callback pattern being used here will make it easy to introduce e.g. a "multi" variant which coerces its values to "bool", "int", "path" etc. 1. 40ea4ed9032 (Add config_error_nonbool() helper function, 2008-02-11) 2. 6c47d0e8f39 (config.c: guard config parser from value=NULL, 2008-02-11). 3. 4c715ebb96a (test-config: add tests for the config_set API, 2014-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 4 +-- builtin/log.c | 2 +- config.c | 63 +++++++++++++++++++++++++++++++--- config.h | 40 +++++++++++++++++++++ pack-bitmap.c | 2 +- submodule.c | 2 +- t/t4202-log.sh | 15 ++++++++ t/t5310-pack-bitmaps.sh | 21 ++++++++++++ t/t7004-tag.sh | 17 +++++++++ t/t7413-submodule-is-active.sh | 16 +++++++++ t/t7900-maintenance.sh | 38 ++++++++++++++++++++ versioncmp.c | 8 ++--- 12 files changed, 214 insertions(+), 14 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 3e94fa5e20f..3fc759b1f0c 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1481,7 +1481,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) if (git_config_lookup_value("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); - if (!git_config_get_knownkey_value_multi(key, &list)) + if (!git_config_get_knownkey_value_multi_string(key, &list)) found = unsorted_string_list_has_string(list, maintpath); if (!found) { @@ -1530,7 +1530,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi usage_with_options(builtin_maintenance_unregister_usage, options); - if (!git_config_get_knownkey_value_multi(key, &list)) + if (!git_config_get_knownkey_value_multi_string(key, &list)) found = unsorted_string_list_has_string(list, maintpath); if (found) { diff --git a/builtin/log.c b/builtin/log.c index 75464c96ccf..d6b1c75ea2e 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -184,7 +184,7 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f struct string_list *include = decoration_filter->include_ref_pattern; const struct string_list *config_exclude; - if (!git_config_get_knownkey_value_multi("log.excludeDecoration", + if (!git_config_get_knownkey_value_multi_string("log.excludeDecoration", &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) diff --git a/config.c b/config.c index 5cd130ddbb9..25bb6514f81 100644 --- a/config.c +++ b/config.c @@ -2428,7 +2428,8 @@ int git_configset_get_value(struct config_set *cs, const char *key, const char * static int git_configset_get_value_multi_1(struct config_set *cs, const char *key, const struct string_list **dest, - int read_only, int knownkey) + int read_only, int knownkey, + string_list_each_func_t check_fn) { struct config_set_element *e; int ret; @@ -2440,28 +2441,51 @@ static int git_configset_get_value_multi_1(struct config_set *cs, const char *ke return ret; else if (!e) return 1; + if (check_fn && + (ret = for_each_string_list(&e->value_list, check_fn, (void *)key))) + return ret; if (!read_only) *dest = &e->value_list; return 0; } +static int check_multi_string(struct string_list_item *item, void *util) +{ + return item->string ? 0 : config_error_nonbool(util); +} + int git_configset_get_value_multi(struct config_set *cs, const char *key, const struct string_list **dest) { - return git_configset_get_value_multi_1(cs, key, dest, 0, 0); + return git_configset_get_value_multi_1(cs, key, dest, 0, 0, NULL); } int git_configset_get_knownkey_value_multi(struct config_set *cs, const char *const key, const struct string_list **dest) { - return git_configset_get_value_multi_1(cs, key, dest, 0, 1); + return git_configset_get_value_multi_1(cs, key, dest, 0, 1, NULL); +} + +int git_configset_get_value_multi_string(struct config_set *cs, const char *key, + const struct string_list **dest) +{ + return git_configset_get_value_multi_1(cs, key, dest, 0, 0, + check_multi_string); +} + +int git_configset_get_knownkey_value_multi_string(struct config_set *cs, + const char *const key, + const struct string_list **dest) +{ + return git_configset_get_value_multi_1(cs, key, dest, 0, 1, + check_multi_string); } int git_configset_lookup_value(struct config_set *cs, const char *key) { - return git_configset_get_value_multi_1(cs, key, NULL, 1, 0); + return git_configset_get_value_multi_1(cs, key, NULL, 1, 0, NULL); } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -2603,7 +2627,8 @@ void repo_config(struct repository *repo, config_fn_t fn, void *data) int repo_config_lookup_value(struct repository *repo, const char *key) { git_config_check_init(repo); - return git_configset_get_value_multi_1(repo->config, key, NULL, 1, 0); + return git_configset_get_value_multi_1(repo->config, key, NULL, 1, 0, + NULL); } int repo_config_get_value(struct repository *repo, @@ -2629,6 +2654,22 @@ int repo_config_get_knownkey_value_multi(struct repository *repo, return git_configset_get_knownkey_value_multi(repo->config, key, dest); } +int repo_config_get_value_multi_string(struct repository *repo, + const char *key, + const struct string_list **dest) +{ + git_config_check_init(repo); + return git_configset_get_value_multi_string(repo->config, key, dest); +} + +int repo_config_get_knownkey_value_multi_string(struct repository *repo, + const char *key, + const struct string_list **dest) +{ + git_config_check_init(repo); + return git_configset_get_knownkey_value_multi_string(repo->config, key, dest); +} + int repo_config_get_string(struct repository *repo, const char *key, char **dest) { @@ -2759,6 +2800,18 @@ int git_config_get_knownkey_value_multi(const char *const key, return repo_config_get_knownkey_value_multi(the_repository, key, dest); } +int git_config_get_value_multi_string(const char *key, + const struct string_list **dest) +{ + return repo_config_get_value_multi_string(the_repository, key, dest); +} + +int git_config_get_knownkey_value_multi_string(const char *key, + const struct string_list **dest) +{ + return repo_config_get_knownkey_value_multi_string(the_repository, key, dest); +} + int git_config_get_string(const char *key, char **dest) { return repo_config_get_string(the_repository, key, dest); diff --git a/config.h b/config.h index cf1ae7862a8..047ef83afc6 100644 --- a/config.h +++ b/config.h @@ -484,6 +484,30 @@ int git_configset_get_knownkey_value_multi(struct config_set *cs, const char *const key, const struct string_list **dest); +/** + * A validation wrapper for git_configset_get_value_multi() which does + * for it what git_configset_get_string() does for + * git_configset_get_value(). + * + * The configuration syntax allows for "[section] key", which will + * give us a NULL entry in the "struct string_list", as opposed to + * "[section] key =" which is the empty string. Most users of the API + * are not prepared to handle NULL in a "struct string_list". + */ +int git_configset_get_value_multi_string(struct config_set *cs, const char *key, + const struct string_list **dest); + +/** + * A wrapper for git_configset_get_value_multi_string() which does for + * it what git_configset_get_knownkey_value_multi() does for + * git_configset_get_value_multi(). + */ +RESULT_MUST_BE_USED +int git_configset_get_knownkey_value_multi_string(struct config_set *cs, + const char *const key, + const struct string_list **dest); + + /** * Clears `config_set` structure, removes all saved variable-value pairs. */ @@ -527,6 +551,14 @@ int repo_config_get_knownkey_value_multi(struct repository *repo, const char *const key, const struct string_list **dest); RESULT_MUST_BE_USED +int repo_config_get_value_multi_string(struct repository *repo, + const char *key, + const struct string_list **dest); +RESULT_MUST_BE_USED +int repo_config_get_knownkey_value_multi_string(struct repository *repo, + const char *const key, + const struct string_list **dest); +RESULT_MUST_BE_USED int repo_config_lookup_value(struct repository *repo, const char *key); int repo_config_get_string(struct repository *repo, const char *key, char **dest); @@ -592,6 +624,14 @@ RESULT_MUST_BE_USED int git_config_get_knownkey_value_multi(const char *const key, const struct string_list **dest); +RESULT_MUST_BE_USED +int git_config_get_value_multi_string(const char *key, + const struct string_list **dest); + +RESULT_MUST_BE_USED +int git_config_get_knownkey_value_multi_string(const char *const key, + const struct string_list **dest); + /** * The same as git_config_value(), except without the extra work to * return the value to the user, used to check if a value for a key diff --git a/pack-bitmap.c b/pack-bitmap.c index 0b4e73abbfa..9a61d9ff9a8 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2303,7 +2303,7 @@ const struct string_list *bitmap_preferred_tips(struct repository *r) { const struct string_list *dest; - if (!repo_config_get_knownkey_value_multi(r, "pack.preferbitmaptips", + if (!repo_config_get_knownkey_value_multi_string(r, "pack.preferbitmaptips", &dest)) return dest; return NULL; diff --git a/submodule.c b/submodule.c index e8c4362743d..f84b253154e 100644 --- a/submodule.c +++ b/submodule.c @@ -274,7 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - if (!repo_config_get_knownkey_value_multi(repo, "submodule.active", &sl)) { + if (!repo_config_get_knownkey_value_multi_string(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 2ce2b41174d..ae73aef922f 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -835,6 +835,21 @@ test_expect_success 'log.decorate configuration' ' ' +test_expect_success 'parse log.excludeDecoration with no value' ' + cp .git/config .git/config.orig && + test_when_finished mv .git/config.orig .git/config && + + cat >>.git/config <<-\EOF && + [log] + excludeDecoration + EOF + cat >expect <<-\EOF && + error: missing value for '\''log.excludeDecoration'\'' + EOF + git log --decorate=short 2>actual && + test_cmp expect actual +' + test_expect_success 'decorate-refs with glob' ' cat >expect.decorate <<-\EOF && Merge-tag-reach diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 6d693eef82f..68195a1de36 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -404,6 +404,27 @@ test_bitmap_cases () { ) ' + test_expect_success 'pack.preferBitmapTips' ' + git init repo && + test_when_finished "rm -rf repo" && + ( + cd repo && + git config pack.writeBitmapLookupTable '"$writeLookupTable"' && + test_commit_bulk --message="%s" 103 && + + cat >>.git/config <<-\EOF && + [pack] + preferBitmapTips + EOF + + cat >expect <<-\EOF && + error: missing value for '\''pack.preferbitmaptips'\'' + EOF + git repack -adb 2>actual && + test_cmp expect actual + ) + ' + test_expect_success 'complains about multiple pack bitmaps' ' rm -fr repo && git init repo && diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 9aa1660651b..f4a31ada79a 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1843,6 +1843,23 @@ test_expect_success 'invalid sort parameter in configuratoin' ' test_must_fail git tag -l "foo*" ' +test_expect_success 'version sort handles empty value for versionsort.{prereleaseSuffix,suffix}' ' + cp .git/config .git/config.orig && + test_when_finished mv .git/config.orig .git/config && + + cat >>.git/config <<-\EOF && + [versionsort] + prereleaseSuffix + suffix + EOF + cat >expect <<-\EOF && + error: missing value for '\''versionsort.suffix'\'' + error: missing value for '\''versionsort.prereleasesuffix'\'' + EOF + git tag -l --sort=version:refname 2>actual && + test_cmp expect actual +' + test_expect_success 'version sort with prerelease reordering' ' test_config versionsort.prereleaseSuffix -rc && git tag foo1.6-rc1 && diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh index 7cdc2637649..887d181b72e 100755 --- a/t/t7413-submodule-is-active.sh +++ b/t/t7413-submodule-is-active.sh @@ -51,6 +51,22 @@ test_expect_success 'is-active works with submodule..active config' ' test-tool -C super submodule is-active sub1 ' +test_expect_success 'is-active handles submodule.active config missing a value' ' + cp super/.git/config super/.git/config.orig && + test_when_finished mv super/.git/config.orig super/.git/config && + + cat >>super/.git/config <<-\EOF && + [submodule] + active + EOF + + cat >expect <<-\EOF && + error: missing value for '\''submodule.active'\'' + EOF + test-tool -C super submodule is-active sub1 2>actual && + test_cmp expect actual +' + test_expect_success 'is-active works with basic submodule.active config' ' test_when_finished "git -C super config submodule.sub1.URL ../sub" && test_when_finished "git -C super config --unset-all submodule.active" && diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 96bdd420456..1201866c8d0 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -505,6 +505,44 @@ test_expect_success 'register and unregister' ' git maintenance unregister --force ' +test_expect_success 'register with no value for maintenance.repo' ' + cp .git/config .git/config.orig && + test_when_finished mv .git/config.orig .git/config && + + cat >>.git/config <<-\EOF && + [maintenance] + repo + EOF + cat >expect <<-\EOF && + error: missing value for '\''maintenance.repo'\'' + EOF + git maintenance register 2>actual && + test_cmp expect actual && + git config maintenance.repo +' + +test_expect_success 'unregister with no value for maintenance.repo' ' + cp .git/config .git/config.orig && + test_when_finished mv .git/config.orig .git/config && + + cat >>.git/config <<-\EOF && + [maintenance] + repo + EOF + cat >expect <<-\EOF && + error: missing value for '\''maintenance.repo'\'' + EOF + test_expect_code 128 git maintenance unregister 2>actual.raw && + grep ^error actual.raw >actual && + test_cmp expect actual && + git config maintenance.repo && + + git maintenance unregister --force 2>actual.raw && + grep ^error actual.raw >actual && + test_cmp expect actual && + git config maintenance.repo +' + test_expect_success !MINGW 'register and unregister with regex metacharacters' ' META="a+b*c" && git init "$META" && diff --git a/versioncmp.c b/versioncmp.c index effe1a6a6be..4efb5f9e621 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -165,11 +165,11 @@ int versioncmp(const char *s1, const char *s2) initialized = 1; prereleases_ret = - git_config_get_knownkey_value_multi("versionsort.suffix", - &prereleases); + git_config_get_knownkey_value_multi_string("versionsort.suffix", + &prereleases); deprecated_prereleases_ret = - git_config_get_knownkey_value_multi("versionsort.prereleasesuffix", - &deprecated_prereleases); + git_config_get_knownkey_value_multi_string("versionsort.prereleasesuffix", + &deprecated_prereleases); if (!prereleases_ret) { if (!deprecated_prereleases_ret) From patchwork Wed Oct 26 15:35:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13020840 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 652D8FA373E for ; Wed, 26 Oct 2022 15:37:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233882AbiJZPhV (ORCPT ); Wed, 26 Oct 2022 11:37:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59802 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234644AbiJZPhN (ORCPT ); Wed, 26 Oct 2022 11:37:13 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3340513331D for ; Wed, 26 Oct 2022 08:36:51 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id o4so18786231wrq.6 for ; Wed, 26 Oct 2022 08:36:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=MjR5MStriQ2BWCyiFta0V5akmF/RpPVp9tflD5MZrQg=; b=LgKyjIfy97Mrgq3S6Mba7e6eghdbQ2PWk3XcahlDcb93b7F4UJbzGypz9Ng6ohjMGi DiV9YR2o9egWSfoCgv8zIKVnp7ydWCgE+ctq7xlgO90YqrTZR/+AEKm840USkFjaQQge eRnCyebS3fL+RMoIii28g5nVDyJDyOogcaGKC+yaBvuc1iiivgOVWFrzGw5PfpNIOFxg VWmlGGH/5+X5FjsuTObenp4S82iUaRxlAVGX5D3jttse044cGvTKD/CffQzQBNzGvE3D r6iap0MFl9qJpUModXPuh3wjM7npZDY7moEEUs/zIcYN2r5IVGp8aJ89qYEfqZOHlqkx xGSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MjR5MStriQ2BWCyiFta0V5akmF/RpPVp9tflD5MZrQg=; b=LozG0NHB1rDA3D3bvbBMifpUiAMXM0KDegguTp5Zk4YieqXDxOZkBLisuBcdiEe6As LgNoo2JESmKpzVbt4ynyzWnUhWax8BAnPOu6TIbvQUfkQ6x2/CfHF6Reewu1dqvMXGOH eYf+xFKODO474AcWrJPHdK5RA7vzesn3ieI00iQwd3kiB1xhmNS+yQKihtoOO5fYeZlo mebvRybit4DzXyzt13jybbFFfLnhMTQd1KVO93L5VftjvxoEWGTmTU4LEh4ilsEpYQo6 B1XNxUBxeW6z7wDrmIFynCCf49rJQwrnpeaz28QYm7AdvJgYXt66oKxBwJZ22X4QBvDm W0dw== X-Gm-Message-State: ACrzQf2IeDYewHU4hGb1jdKJnF5rs9wr1oUtct5GwWniH6tRzpNoSazd 9sk3sHxZftkzn1FSPp/3O8gJp5T5ovZNjg== X-Google-Smtp-Source: AMsMyM55zis9UakqjB3OSY6vTjPsemaneCU8+juRyn5xDHOp8IrvWnXa3vLo9nw2+LOlyzp9BtrPPg== X-Received: by 2002:a05:6000:1562:b0:231:1b02:3dbb with SMTP id 2-20020a056000156200b002311b023dbbmr29425081wrz.685.1666798609220; Wed, 26 Oct 2022 08:36:49 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id r1-20020a0560001b8100b002366f300e57sm5581884wru.23.2022.10.26.08.36.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 08:36:48 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH 10/10] for-each-repo: with bad config, don't conflate and Date: Wed, 26 Oct 2022 17:35:23 +0200 Message-Id: X-Mailer: git-send-email 2.38.0.1251.g3eefdfb5e7a In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix a logic error in 4950b2a2b5c (for-each-repo: run subcommands on configured repos, 2020-09-11). Due to assuming that elements returned from the repo_config_get_value_multi() call wouldn't be "NULL" we'd conflate the and part of the argument list when running commands. As noted in the preceding commit the fix is to move to a safer "*_multi_string()" version of the *__multi() API. This change is separated from the rest because those all segfaulted. In this change we ended up with different behavior. When using the "--config=" form we take each element of the list as a path to a repository. E.g. with a configuration like: [repo] list = /some/repo We would, with this command: git for-each-repo --config=repo.list status builtin Run a "git status" in /some/repo, as: git -C /some/repo status builtin I.e. ask "status" to report on the "builtin" directory. But since a configuration such as this would result in a "struct string_list *" with one element, whose "string" member is "NULL": [repo] list We would, when constructing our command-line in "builtin/for-each-repo.c"... strvec_pushl(&child.args, "-C", path, NULL); for (i = 0; i < argc; i++) strvec_push(&child.args, argv[i]); ...have that "path" be "NULL", and as strvec_pushl() stops when it sees NULL we'd end with the first "argv" element as the argument to the "-C" option, e.g.: git -C status builtin I.e. we'd run the command "builtin" in the "status" directory. In another context this might be an interesting security vulnerability, but I think that this amounts to a nothingburger on that front. A hypothetical attacker would need to be able to write config for the victim to run, if they're able to do that there's more interesting attack vectors. See the "safe.directory" facility added in 8d1a7448206 (setup.c: create `safe.bareRepository`, 2022-07-14). An even more unlikely possibility would be an attacker able to generate the config used for "for-each-repo --config=", but nothing else (e.g. an automated system producing that list). Even in that case the attack vector is limited to the user running commands whose name matches a directory that's interesting to the attacker (e.g. a "log" directory in a repository). The second argument (if any) of the command is likely to make git die without doing anything interesting (e.g. "-p" to "log", there being no "-p" built-in command to run). Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 2 +- t/t0068-for-each-repo.sh | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 16e9a76d04a..125901f2fc0 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -43,7 +43,7 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) if (!config_key) die(_("missing --config=")); - err = repo_config_get_value_multi(the_repository, config_key, &values); + err = repo_config_get_value_multi_string(the_repository, config_key, &values); if (err < 0) usage_msg_optf(_("got bad config --config=%s"), for_each_repo_usage, options, config_key); diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index 115221c9ca5..c27d4dc5f71 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -39,4 +39,17 @@ test_expect_success 'error on bad config keys' ' test_expect_code 129 git for-each-repo --config="'\''.b" ' +test_expect_success 'error on NULL value for config keys' ' + cat >>.git/config <<-\EOF && + [empty] + key + EOF + cat >expect <<-\EOF && + error: missing value for '\''empty.key'\'' + EOF + test_expect_code 129 git for-each-repo --config=empty.key 2>actual.raw && + grep ^error actual.raw >actual && + test_cmp expect actual +' + test_done