From patchwork Thu Feb 2 13:27:13 2023 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: 13126020 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 09EDAC61DA4 for ; Thu, 2 Feb 2023 13:27:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232059AbjBBN1x (ORCPT ); Thu, 2 Feb 2023 08:27:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58322 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229916AbjBBN1w (ORCPT ); Thu, 2 Feb 2023 08:27:52 -0500 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 10B718C42F for ; Thu, 2 Feb 2023 05:27:31 -0800 (PST) Received: by mail-wm1-x32a.google.com with SMTP id n28-20020a05600c3b9c00b003ddca7a2bcbso1386116wms.3 for ; Thu, 02 Feb 2023 05:27:30 -0800 (PST) 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=dRW7mHY9/QvX8zcbl7zdiGKNV8MYDAFfHdG9TjFWy2E=; b=oSFBEb8BusRSTMOU2TRMSe32Kjvg/+5bl0BQQ+PhgMHvXKKA7Ft7bpAN9200S34wCG HXULKCn4eDE+7zjj5dEUC3rQ/Jf2Mpuov4yAoIaGOQvN7GpkJ9G/dnWYWX4Zj4jnsd+/ 3Kva/P5zFY9zibC02/0AZZpFj8IdqncQh84JAokwDBguKRt98jAikaNYGwht29ElXw19 PuHDrujdGNYVZpdIVqwWH17MjpD9/ISAQ4DpQJ+1p8mFQEHfDJSeaSmRosI2JzwUm2yY Je3nEoCijcJ2bqoajy33oCPlHPgSTu3h/5UtA7NGr8ftzGzLM9WTJ0/l8lLnvH33AX+u n3sQ== 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=dRW7mHY9/QvX8zcbl7zdiGKNV8MYDAFfHdG9TjFWy2E=; b=fSK+5ZE/OiMSmMwPoVfM/eYbbOan3H5aebUikk3ptLi43989hZkzRxrK95uKa83tWJ tlebzgyswDLH/YE9vKEb/GKUIl4z1TMWuL5pYb6JUSkY5hBjfM2sSW0BBo7hWoPu6kW9 k1ltLcgf1rhpiX2jncQhqhuDC8KCYG/sq/1BqYEcfHAjWBSUnN2lgR9bdna+ndjGOjYI 6gJbQa1Dm5jsBDZnruHzAD73OrlAi/YQV2wdPuPOkQWxymAIoT/nSqepLM7BJUB/fPQD l0p/FNw4lzPHBZNagX1XqfI0y266U928PD12PA/IlCKRPmyEtgEVuCAcAESbWaYrkMfR RsdQ== X-Gm-Message-State: AO0yUKUbPc/8N9NHc0YSNDYhejGn6JPJZr7Gv2ytRRjDV5CMngbY1/8f 3aLSpNJK0z7FrcfTkaDUoVfE85jsKLBV2k/r X-Google-Smtp-Source: AK7set+679Hv6A0H1uRtJ+uf6ml1xnYvvUifNQ9lmAW+jNpPtHFFBFUq4KnKKumjAmMOPmcPGIAOqw== X-Received: by 2002:a05:600c:4f8d:b0:3dc:e66:4cb9 with SMTP id n13-20020a05600c4f8d00b003dc0e664cb9mr5982589wmq.13.1675344449238; Thu, 02 Feb 2023 05:27:29 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:27 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 1/9] config tests: cover blind spots in git_die_config() tests Date: Thu, 2 Feb 2023 14:27:13 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org There were no tests checking for the output of the git_die_config() function in the config API, added in 5a80e97c827 (config: add `git_die_config()` to the config-set API, 2014-08-07). We only tested "test_must_fail", but didn't assert the output. We need tests for this because a subsequent commit will alter the return value of git_config_get_value_multi(), which is used to get the config values in the git_die_config() function. This test coverage helps to build confidence in that subsequent change. These tests cover different interactions with git_die_config(): - The "notes.mergeStrategy" test in "t/t3309-notes-merge-auto-resolve.sh" is a case where a function outside of config.c (git_config_get_notes_strategy()) calls git_die_config(). - The "gc.pruneExpire" test in "t5304-prune.sh" is a case where git_config_get_expiry() calls git_die_config(), covering a different "type" than the "string" test for "notes.mergeStrategy". - The "fetch.negotiationAlgorithm" test in "t/t5552-skipping-fetch-negotiator.sh" is a case where git_config_get_string*() calls git_die_config(). We also cover both the "from command-line config" and "in file..at line" cases here. The clobbering of existing ".git/config" files here is so that we're not implicitly testing the line count of the default config. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t3309-notes-merge-auto-resolve.sh | 7 ++++++- t/t5304-prune.sh | 12 ++++++++++-- t/t5552-skipping-fetch-negotiator.sh | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/t/t3309-notes-merge-auto-resolve.sh b/t/t3309-notes-merge-auto-resolve.sh index 141d3e4ca4d..9bd5dbf341f 100755 --- a/t/t3309-notes-merge-auto-resolve.sh +++ b/t/t3309-notes-merge-auto-resolve.sh @@ -360,7 +360,12 @@ test_expect_success 'merge z into y with invalid strategy => Fail/No changes' ' test_expect_success 'merge z into y with invalid configuration option => Fail/No changes' ' git config core.notesRef refs/notes/y && - test_must_fail git -c notes.mergeStrategy="foo" notes merge z && + cat >expect <<-\EOF && + error: unknown notes merge strategy foo + fatal: unable to parse '\''notes.mergeStrategy'\'' from command-line config + EOF + test_must_fail git -c notes.mergeStrategy="foo" notes merge z 2>actual && + test_cmp expect actual && # Verify no changes (y) verify_notes y y ' diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index d65a5f94b4b..5500dd08426 100755 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -72,8 +72,16 @@ test_expect_success 'gc: implicit prune --expire' ' ' test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' ' - git config gc.pruneExpire invalid && - test_must_fail git gc + test_when_finished "rm -rf repo" && + git init repo && + >repo/.git/config && + git -C repo config gc.pruneExpire invalid && + cat >expect <<-\EOF && + error: Invalid gc.pruneexpire: '\''invalid'\'' + fatal: bad config variable '\''gc.pruneexpire'\'' in file '\''.git/config'\'' at line 2 + EOF + test_must_fail git -C repo gc 2>actual && + test_cmp expect actual ' test_expect_success 'gc: start with ok gc.pruneExpire' ' diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh index 165427d57e5..b55a9f65e6b 100755 --- a/t/t5552-skipping-fetch-negotiator.sh +++ b/t/t5552-skipping-fetch-negotiator.sh @@ -3,6 +3,22 @@ test_description='test skipping fetch negotiator' . ./test-lib.sh +test_expect_success 'fetch.negotiationalgorithm config' ' + test_when_finished "rm -rf repo" && + git init repo && + cat >repo/.git/config <<-\EOF && + [fetch] + negotiationAlgorithm + EOF + cat >expect <<-\EOF && + error: missing value for '\''fetch.negotiationalgorithm'\'' + fatal: bad config variable '\''fetch.negotiationalgorithm'\'' in file '\''.git/config'\'' at line 2 + EOF + test_expect_code 128 git -C repo fetch >out 2>actual && + test_must_be_empty out && + test_cmp expect actual +' + have_sent () { while test "$#" -ne 0 do From patchwork Thu Feb 2 13:27:14 2023 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: 13126021 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 D4366C05027 for ; Thu, 2 Feb 2023 13:27:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232356AbjBBN1z (ORCPT ); Thu, 2 Feb 2023 08:27:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58326 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231485AbjBBN1x (ORCPT ); Thu, 2 Feb 2023 08:27:53 -0500 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 574F78F24B for ; Thu, 2 Feb 2023 05:27:32 -0800 (PST) Received: by mail-wm1-x334.google.com with SMTP id j29-20020a05600c1c1d00b003dc52fed235so1398712wms.1 for ; Thu, 02 Feb 2023 05:27:32 -0800 (PST) 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=BIVfkWFemhCYepVOzHTkqvP7n8I4Nt6Qxao618nS0R8=; b=nn5Da2qDA1wmTIefJpK+QepMrwHIAFh+UXGULUgc/huWDFdggtKnH0P9LxIdxZ45rh pukU5rUUIrkgMFMCS7eP2JJqRDDlEmF27lKUk25jzmV+qVE0q9QDi6yEo7AyX6gw8F2h XHFPN28abdOqBNiAdsov8zY/ve2PspzJ1VzGVt9d0Cs2TQSVGbTgQETKk8b7z/PoNUd/ TMVVmlU47dcxXNnO4h2RtMmGW15TG/hUV4Vf6wsHMsFN6SRmygsQeDdlclDwNcmZaIew VolZPCPlrscN2FlDeBBDVNnq3zCM968eIbyDxwVNCHpINKN+UGBjTKS2VEJSjYblcieu X9rQ== 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=BIVfkWFemhCYepVOzHTkqvP7n8I4Nt6Qxao618nS0R8=; b=jZ3sQieX6wZ8lw5WeZvlLk/9mhUzVzOGFP1TsGVdjEGNUyfS7j42hTEZXgUJIwd8cR 1x09BF/Dze4/fDSRv+Bj0pIbGZ4FR56mxvPbvq2J+kFYfiMh/TzisZ9LAYdt2Gvfs0aQ nZ+AUe1kXRcgM/KMuMEtNDq7wRMLVRmscSYbPPll4rI2dXEqjn36m0/qxUgpCEmeldyh v1cRHGXVaodJXfLGuGCd+1ZxCi0kZu41ePoeOEjPlqdp0wDpWT+pEWWKrzLatv6EN1mt 0HlRfMTsgKgvBE/KHR8VJOLX8lXWvJg4cam++eLd90uY3qCwJo8IXycGRhgDsobkjLqw VHSw== X-Gm-Message-State: AO0yUKVV7llTW9T2pz0NLw3BbPiCysRllryfrldk+K0AaLWypyOjrwa4 w0byhl0aihv9MExc6GKTKo6QKJIksYzrgcgQ X-Google-Smtp-Source: AK7set+k7b/V6DjJt6M+0cI1PX2rzckhkfRz0vkC83yvyy5J4ucLouG7R5eIW+kqZudydPrU75y/5g== X-Received: by 2002:a05:600c:3ba9:b0:3dd:daac:d99d with SMTP id n41-20020a05600c3ba900b003dddaacd99dmr5658658wms.36.1675344450516; Thu, 02 Feb 2023 05:27:30 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:29 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 2/9] config tests: add "NULL" tests for *_get_value_multi() Date: Thu, 2 Feb 2023 14:27:14 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d 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 parts of the config_set API were tested for in [1] they didn't add coverage for 3/4 of the "(NULL)" cases handled in "t/helper/test-config.c". We'd test that case for "get_value", but not "get_value_multi", "configset_get_value" and "configset_get_value_multi". We now cover all of those cases, which in turn expose the details of how this part of the config API works. 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 | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index b38e158d3b2..b172565f92a 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -146,6 +146,47 @@ test_expect_success 'find multiple values' ' check_config get_value_multi case.baz sam bat hask ' +test_NULL_in_multi () { + local op="$1" && + local file="$2" && + + test_expect_success "$op: NULL value in config${file:+ in $file}" ' + config="$file" && + if test -z "$config" + then + config=.git/config && + test_when_finished "mv $config.old $config" && + mv "$config" "$config".old + fi && + + cat >"$config" <<-\EOF && + [a]key=x + [a]key + [a]key=y + EOF + case "$op" in + *_multi) + cat >expect <<-\EOF + x + (NULL) + y + EOF + ;; + *) + cat >expect <<-\EOF + y + EOF + ;; + esac && + test-tool config "$op" a.key $file >actual && + test_cmp expect actual + ' +} + +test_NULL_in_multi "get_value_multi" +test_NULL_in_multi "configset_get_value" "my.config" +test_NULL_in_multi "configset_get_value_multi" "my.config" + test_expect_success 'find value from a configset' ' cat >config2 <<-\EOF && [case] From patchwork Thu Feb 2 13:27:15 2023 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: 13126024 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 2671AC05027 for ; Thu, 2 Feb 2023 13:28:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232491AbjBBN2L (ORCPT ); Thu, 2 Feb 2023 08:28:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232375AbjBBN14 (ORCPT ); Thu, 2 Feb 2023 08:27:56 -0500 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C25A98F24E for ; Thu, 2 Feb 2023 05:27:33 -0800 (PST) Received: by mail-wr1-x434.google.com with SMTP id t7so1710043wrp.5 for ; Thu, 02 Feb 2023 05:27:33 -0800 (PST) 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=JWjSMZz/1eHAlpYGbwX6udlZYAB8VFzKdV79RO01ifg=; b=JQypr7NALcHwe+AMD/mvshio4h/326+8+S8MvxgHm6wepjRnElJyNCqPdPTTJcl3+d qMRWpJes4AaPoh1ZkTkVJ+FxItNB1RpqDgUC4/+w8kzPU86oaCQfmNC1rI9hGvkG9fdA Igf9wiIvINt5J9b3PSVrsISQacW/1JtOu3m0V6hRpAXKkUNi9m7NuGYKuDJxb3i6f9D+ kxNd6oYH7XjQawia01n07JyqKIwvSSw5imkIxgPOokZ0twNemLoizvxwoZyu2LZuYf2a ZwK6QXj2rh7RZSdpBx3lPawwtn/aZ0bSD+k4ipSPbPY5Kmu0Cb+n3XKtqlK7CMmbulLi mJFQ== 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=JWjSMZz/1eHAlpYGbwX6udlZYAB8VFzKdV79RO01ifg=; b=E185ZKmQ6unlbCWnGb3f+fVwt4YujG2dyF9cu7bhJ2HLHxhIg0Vo+yHaJDP2Z21tU4 KmkeVwB7trqiCd8im7KylnFOZayoFMkCQXndilco2bhkEULVcezESlVtM6nLzYKhnZVd j/jmSL1LOL7IQsvZfoL2rapXtb8pleGdzVRsFt/cwlYZ9+CP1yUgU9wYYDD0ch2dwSSk 7S1ydN6p6qCwm+YjIKmmWON4Uxapp6bdJLFxa25MCw71uocIsjuxk0e4+767vBJYUWHX tT0zZrMvhhjVgnyq738FH8lfkASpVvxCk67OQJ+WtBvJgNL2+7lE0OVY+LRpDa1kLrYU 8GAw== X-Gm-Message-State: AO0yUKUtenUcg+2s93X3Z94U10dSGNALOhGUfqTC1XGVcjPafE2+e13B G/VNG/+fTREqzesR+o7diUC01GWPrsSNaafH X-Google-Smtp-Source: AK7set/ORy0L0gzFDmuhlMbQUc1zmVWPdQ5pEqnPd1QmDcsH/y+vfzD3W5weAYnGmbWuwykm8ZkVkw== X-Received: by 2002:a05:6000:15cd:b0:2be:d03:287 with SMTP id y13-20020a05600015cd00b002be0d030287mr7291508wry.44.1675344451791; Thu, 02 Feb 2023 05:27:31 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:31 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 3/9] config API: add and use a "git_config_get()" family of functions Date: Thu, 2 Feb 2023 14:27:15 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We already have the basic "git_config_get_value()" function and its "repo_*" and "configset" siblings to get a given "key" and assign the last key found to a provided "value". But some callers don't care about that value, but just want to use the return value of the "get_value()" function to check whether the key exist (or another non-zero return value). The immediate motivation for this is that a subsequent commit will need to change all callers of the "*_get_value_multi()" family of functions. In two cases here we (ab)used it to check whether we had any values for the given key, but didn't care about the return value. The rest of the callers here used various other config API functions to do the same, all of which resolved to the same underlying functions to provide the answer. 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. Having an "unused" value that we throw away internal to config.c is cheap. Another name for this function could have been "*_config_key_exists()", as suggested in [1]. That would work for all of these callers, and would currently be equivalent to this function, as the git_configset_get_value() API normalizes all non-zero return values to a "1". But adding that API would set us up to lose information, as e.g. if git_config_parse_key() in the underlying configset_find_element() fails we'd like to return -1, not 1. Let's change the underlying configset_find_element() function to support this use-case, we'll make further use of it in a subsequent commit where the git_configset_get_value_multi() function itself will expose this new return value. 1. https://lore.kernel.org/git/xmqqczadkq9f.fsf@gitster.g/ Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 5 +--- builtin/submodule--helper.c | 7 +++--- builtin/worktree.c | 3 +-- config.c | 50 +++++++++++++++++++++++++++++++------ config.h | 19 +++++++++++++- 5 files changed, 66 insertions(+), 18 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 02455fdcd73..e38d1783f30 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1493,7 +1493,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(); struct string_list_item *item; const struct string_list *list; @@ -1508,9 +1507,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_get("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); list = git_config_get_value_multi(key); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 4c173d8b37a..2278e8c91cb 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -557,7 +557,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("submodule.active")) module_list_active(&list); info.prefix = prefix; @@ -2743,7 +2743,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("submodule.active")) module_list_active(&list); info.prefix = opt.prefix; @@ -3140,7 +3140,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; @@ -3185,7 +3184,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_get("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 f51c40f1e1e..6ba42d4ad20 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -319,7 +319,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) || @@ -338,7 +337,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_get(&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 00090a32fc3..b88da70c664 100644 --- a/config.c +++ b/config.c @@ -2289,23 +2289,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) + 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) @@ -2314,8 +2319,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) + 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. @@ -2425,8 +2433,25 @@ int git_configset_get_value(struct config_set *cs, const char *key, const char * const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key) { - struct config_set_element *e = configset_find_element(cs, key); - return e ? &e->value_list : NULL; + struct config_set_element *e; + + if (configset_find_element(cs, key, &e)) + return NULL; + else if (!e) + return NULL; + return &e->value_list; +} + +int git_configset_get(struct config_set *cs, const char *key) +{ + struct config_set_element *e; + int ret; + + if ((ret = configset_find_element(cs, key, &e))) + return ret; + else if (!e) + return 1; + return 0; } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -2565,6 +2590,12 @@ void repo_config(struct repository *repo, config_fn_t fn, void *data) configset_iter(repo->config, fn, data); } +int repo_config_get(struct repository *repo, const char *key) +{ + git_config_check_init(repo); + return git_configset_get(repo->config, key); +} + int repo_config_get_value(struct repository *repo, const char *key, const char **value) { @@ -2679,6 +2710,11 @@ void git_config_clear(void) repo_config_clear(the_repository); } +int git_config_get(const char *key) +{ + return repo_config_get(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 ef9eade6414..04c5e594015 100644 --- a/config.h +++ b/config.h @@ -471,9 +471,12 @@ void git_configset_clear(struct config_set *cs); /* * These functions return 1 if not found, and 0 if found, leaving the found - * value in the 'dest' pointer. + * value in the 'dest' pointer (if any). */ +RESULT_MUST_BE_USED +int git_configset_get(struct config_set *cs, const char *key); + /* * Finds the highest-priority value for the configuration variable `key` * and config set `cs`, stores the pointer to it in `value` and returns 0. @@ -494,6 +497,14 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha /* Functions for reading a repository's config */ struct repository; void repo_config(struct repository *repo, config_fn_t fn, void *data); + +/** + * Run only the discover part of the repo_config_get_*() functions + * below, in addition to 1 if not found, returns negative values on + * error (e.g. if the key itself is invalid). + */ +RESULT_MUST_BE_USED +int repo_config_get(struct repository *repo, const char *key); 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, @@ -530,8 +541,14 @@ void git_protected_config(config_fn_t fn, void *data); * manner, the config API provides two functions `git_config_get_value` * and `git_config_get_value_multi`. They both read values from an internal * cache generated previously from reading the config files. + * + * For those git_config_get*() functions that aren't documented, + * consult the corresponding repo_config_get*() function's + * documentation. */ +int git_config_get(const char *key); + /** * Finds the highest-priority value for the configuration variable `key`, * stores the pointer to it in `value` and returns 0. When the From patchwork Thu Feb 2 13:27:16 2023 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: 13126023 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 9F2ABC05027 for ; Thu, 2 Feb 2023 13:28:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231345AbjBBN17 (ORCPT ); Thu, 2 Feb 2023 08:27:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58342 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232297AbjBBN1y (ORCPT ); Thu, 2 Feb 2023 08:27:54 -0500 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E85A08F507 for ; Thu, 2 Feb 2023 05:27:34 -0800 (PST) Received: by mail-wm1-x332.google.com with SMTP id bg13-20020a05600c3c8d00b003d9712b29d2so3728811wmb.2 for ; Thu, 02 Feb 2023 05:27:34 -0800 (PST) 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=4fML4UxGMWMWjtU6Ucb7Zg8yKYyLsNwP0LQBotHh9Uk=; b=E8U9XKBv+y0OI+XaiW7D+CwK4FkfeP56U76sVwuuiKD102Gc4o+Gau6FG9rDTlLwRy Ba32Ex9hjl/GUNSexj8DE++cqolZ1jhNAMuRadipga4AKuskSRJeC6lzL0yU8HQ+HBDH ggBTh0qECF3p5zwnd3nBxncNH00m3fR19s5x1clD3DuMmp7GFkbVaJKJw1O7oi6iZxJ0 fPd+Vd6okGUYVlkz4OgN6bD9SnjyvtjCP5YPSKVdJJ1qPFYywMyyYdaDn4xR3SLsfR6Z XKhJjdfnQptl8htw+e8eqmR8gvIv3s79SRm/tioCx9mEW/4yqgZvMoytdDyItEdBWo9E xa9Q== 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=4fML4UxGMWMWjtU6Ucb7Zg8yKYyLsNwP0LQBotHh9Uk=; b=7yJwzvkSktnNzTihP+IoS4LEb2pDxa0yX3xpyPloFxlRPNo+tpBK5qAcgOrkZDlXYD KglHTv0m+IJrroX4dCoD21RMu+g3hW7mmr6tGvvqMqfp77c9FhsUA4gnc4dvm8Gz5O0f +f27buMoB2djfCPhRJLjsH9hF5vB9u+QSvHr7/2NSf9S/RtDWKJHW1W+iaLQG+uP9gH0 EtZUX/C5gvDFWrFsGO1lwJSE23oUD857CVkbXn/f864Je1E9DboVXYytlB4VzpB4OAzt v6h5tuwPzCDxYyK3t8fMPt+DF6ntRmGTXJENuj/MxIlJ2x7SSq7wrYmEMCVcGAouDDaE d0aQ== X-Gm-Message-State: AO0yUKWzHTzagXSn/0lioExW1I836FunHqAOqWtK7KciyqPqJ7w9ziSO il2lH2RN6ozuPMGL9sRxZ/uWPZCl7oU3aQyU X-Google-Smtp-Source: AK7set+OPVPgX1nCn4TAFE8oqzsJh8O9WczdiqdPN4vFOt4lx0L1CoqrHGJjXRFZxtdMLN0kFG1E3Q== X-Received: by 2002:a05:600c:4a9b:b0:3de:1d31:1043 with SMTP id b27-20020a05600c4a9b00b003de1d311043mr6017488wmp.21.1675344453082; Thu, 02 Feb 2023 05:27:33 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:32 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 4/9] versioncmp.c: refactor config reading next commit Date: Thu, 2 Feb 2023 14:27:16 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Refactor the reading of the versionSort.suffix and versionSort.prereleaseSuffix configuration variables to stay within the bounds of our CodingGuidelines when it comes to line length, and to avoid repeating ourselves. Let's also split out the names of the config variables into variables of our own, so we don't have to repeat ourselves, and refactor the nested if/else to avoid indenting it, and the existing bracing style issue. This all helps with the subsequent commit, where we'll need to start checking different git_config_get_value_multi() return value. See c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) for the original implementation of most of this. Moving the "initialized = 1" assignment allows us to move some of this to the variable declarations in the subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason --- versioncmp.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/versioncmp.c b/versioncmp.c index 069ee94a4d7..323f5d35ea8 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -160,15 +160,18 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { - const struct string_list *deprecated_prereleases; + const char *const newk = "versionsort.suffix"; + const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *oldl; + + prereleases = git_config_get_value_multi(newk); + oldl = git_config_get_value_multi(oldk); + if (prereleases && oldl) + warning("ignoring %s because %s is set", oldk, newk); + else if (!prereleases) + prereleases = oldl; + initialized = 1; - prereleases = git_config_get_value_multi("versionsort.suffix"); - deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); - if (prereleases) { - if (deprecated_prereleases) - warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); - } else - prereleases = deprecated_prereleases; } if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1, &diff)) From patchwork Thu Feb 2 13:27:17 2023 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: 13126026 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 35A0FC05027 for ; Thu, 2 Feb 2023 13:28:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231766AbjBBN2P (ORCPT ); Thu, 2 Feb 2023 08:28:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59262 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231903AbjBBN2J (ORCPT ); Thu, 2 Feb 2023 08:28:09 -0500 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6A5D38F510 for ; Thu, 2 Feb 2023 05:27:36 -0800 (PST) Received: by mail-wm1-x336.google.com with SMTP id hn2-20020a05600ca38200b003dc5cb96d46so3713514wmb.4 for ; Thu, 02 Feb 2023 05:27:36 -0800 (PST) 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=JDq5pvU7qd9iE1C7DXXGheajDGhhjIcxWATy/a/uV9s=; b=EEzd4Mt58DW/Fh+mNC+RG0WKaY3XRyqTRfKgghQT8g9FTN54j8nMOOtZ6MLbkVjPW+ r85Qu0fq/a3qzmyjZa9swck6fI1v8zXHAXsPhtRz9XWIXtzA0WrhkOYlB3EEY7WLufff W/9A31hRXp873yfLb0GIgYdR+PhbtB8dbpeGXlg7k4x3MgHuYce5zBpPTxiDOupgT0nS TrjaFtBs+L2BE338wJtu+TJD8nyzBWxe7lTz1CE2hGV9N2RddOgRwuDQ0RtwEeljrOOQ 99RYzbl31QJURNP1jDURb7MFLgVcpKP5WTNhexStfFodl8RixZYJlwb863wU6OYgsUWN n0bQ== 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=JDq5pvU7qd9iE1C7DXXGheajDGhhjIcxWATy/a/uV9s=; b=pjfCw+8DwrnCZqfOs2Z879cN7s3wah/fKxPn29uzUN7Ac+5XJqxqgLfWsEdFfSBXrP 5RPkkajMXYBceW7SWO5PAearnGP+U5RN4DmVYwicMdkgDTKd0zH9BY1pCLshKw1LXRS9 yt0E3/0lRt7y1DVQHxJyvI2UdcPxAOmnFrToha/LygsLiFKW4SfcA5vckZXekJet3fnv joV2VPbg3ipWbtX5hu5RhoUb6TTfDVZbiCxVkKq9EX5MFKrE+Bq/XseDspszpCEmeoEo /p5zign4rxdXposGgueEskiZ45u4rgaX9N93E7pZk3/81N8fdNcZRkE9OIQDTxK28YZb Ij9Q== X-Gm-Message-State: AO0yUKVU/v33r0RsnWnJLjDxa4Qz+X1AHpDm3TfwV2jU0P0VWztT23v6 pVeM0sXVxSkEWO28YVNqJdd7lEbaxfwcMtBn X-Google-Smtp-Source: AK7set/kvHKODQQbYZVRZv6TAiFOUEc5sPSqo+8ZNVUcOPjaQcs0OOJUtwFUGAXfIDuUqLgJO4ZnhQ== X-Received: by 2002:a7b:c4c9:0:b0:3de:1d31:1042 with SMTP id g9-20020a7bc4c9000000b003de1d311042mr6560497wmk.23.1675344454486; Thu, 02 Feb 2023 05:27:34 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:33 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 5/9] config API: have *_multi() return an "int" and take a "dest" Date: Thu, 2 Feb 2023 14:27:17 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Have the "git_configset_get_value_multi()" function and its siblings return an "int" and populate a "**dest" parameter like every other git_configset_get_*()" in the API. As we'll see in in subsequent commits this fixes a blind spot in the API where it wasn't possible to tell whether a list was empty from whether a config key existed. We'll take advantage of that in subsequent commits, but for now we're faithfully converting existing API callers. 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. git_configset_get_int() returns "1" rather than ferrying up the "-1" that "git_configset_get_value()" might return, but that's not being done in this series Most of this is straightforward, commentary on cases that stand out: - To ensure that we'll properly use the return values of this function in the future we're using the "RESULT_MUST_BE_USED" macro introduced in [1]. As git_die_config() now has to handle this return value let's have it BUG() if it can't find the config entry. As tested for in a preceding commit we can rely on getting the config list in git_die_config(). - The loops after getting the "list" value in "builtin/gc.c" could also make use of "unsorted_string_list_has_string()" instead of using that loop, but let's leave that for now. - In "versioncmp.c" we now use the return value of the functions, instead of checking if the lists are still non-NULL. 1. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01), Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 5 +---- builtin/gc.c | 10 ++++------ builtin/log.c | 6 +++--- config.c | 34 ++++++++++++++++++++-------------- config.h | 29 +++++++++++++++++++++-------- pack-bitmap.c | 6 +++++- submodule.c | 3 +-- t/helper/test-config.c | 6 ++---- versioncmp.c | 11 +++++++---- 9 files changed, 64 insertions(+), 46 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 6aeac371488..fd0e7739e6a 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -45,14 +45,11 @@ 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); - /* * Do nothing on an empty list, which is equivalent to the case * where the config variable does not exist at all. */ - if (!values) + if (repo_config_get_value_multi(the_repository, config_key, &values)) return 0; for (i = 0; !result && i < values->nr; i++) diff --git a/builtin/gc.c b/builtin/gc.c index e38d1783f30..2b3da377d52 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1510,8 +1510,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) if (git_config_get("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); - list = git_config_get_value_multi(key); - if (list) { + if (!git_config_get_value_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1577,11 +1576,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi if (config_file) { git_configset_init(&cs); git_configset_add_file(&cs, config_file); - list = git_configset_get_value_multi(&cs, key); - } else { - list = git_config_get_value_multi(key); } - if (list) { + if (!(config_file + ? git_configset_get_value_multi(&cs, key, &list) + : git_config_get_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 04412dd9c93..cec8cabd21e 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_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/config.c b/config.c index b88da70c664..ce5d50a490c 100644 --- a/config.c +++ b/config.c @@ -2417,29 +2417,34 @@ int git_configset_add_file(struct config_set *cs, const char *filename) 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); + if ((ret = git_configset_get_value_multi(cs, key, &values))) + return ret; - 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) +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest) { struct config_set_element *e; + int ret; - if (configset_find_element(cs, key, &e)) - return NULL; + if ((ret = configset_find_element(cs, key, &e))) + return ret; else if (!e) - return NULL; - return &e->value_list; + return 1; + *dest = &e->value_list; + + return 0; } int git_configset_get(struct config_set *cs, const char *key) @@ -2603,11 +2608,11 @@ 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_string(struct repository *repo, @@ -2720,9 +2725,9 @@ 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); + return repo_config_get_value_multi(the_repository, key, dest); } int git_config_get_string(const char *key, char **dest) @@ -2869,7 +2874,8 @@ void git_die_config(const char *key, const char *err, ...) error_fn(err, params); va_end(params); } - values = git_config_get_value_multi(key); + if (git_config_get_value_multi(key, &values)) + BUG("for key '%s' we must have a value to report on", 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 04c5e594015..fbc153cdc96 100644 --- a/config.h +++ b/config.h @@ -459,10 +459,18 @@ 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. + * 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. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. */ -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key); +RESULT_MUST_BE_USED +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest); /** * Clears `config_set` structure, removes all saved variable-value pairs. @@ -507,8 +515,9 @@ RESULT_MUST_BE_USED int repo_config_get(struct repository *repo, const char *key); 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); +RESULT_MUST_BE_USED +int repo_config_get_value_multi(struct repository *repo, const char *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, @@ -561,10 +570,14 @@ 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. */ -const struct string_list *git_config_get_value_multi(const char *key); +RESULT_MUST_BE_USED +int git_config_get_value_multi(const char *key, + const struct string_list **dest); /** * Resets and invalidates the config cache. diff --git a/pack-bitmap.c b/pack-bitmap.c index d2a42abf28c..15c5eb507c0 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2314,7 +2314,11 @@ 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_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 3a0dfc417c0..4b6f5223b0c 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_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..8f70beb6c9d 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_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_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 323f5d35ea8..60c3a517122 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -162,13 +162,16 @@ int versioncmp(const char *s1, const char *s2) if (!initialized) { const char *const newk = "versionsort.suffix"; const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *newl; const struct string_list *oldl; + int new = git_config_get_value_multi(newk, &newl); + int old = git_config_get_value_multi(oldk, &oldl); - prereleases = git_config_get_value_multi(newk); - oldl = git_config_get_value_multi(oldk); - if (prereleases && oldl) + if (!new && !old) warning("ignoring %s because %s is set", oldk, newk); - else if (!prereleases) + if (!new) + prereleases = newl; + else if (!old) prereleases = oldl; initialized = 1; From patchwork Thu Feb 2 13:27:18 2023 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: 13126025 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 15685C636D4 for ; Thu, 2 Feb 2023 13:28:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232435AbjBBN2N (ORCPT ); Thu, 2 Feb 2023 08:28:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232089AbjBBN15 (ORCPT ); Thu, 2 Feb 2023 08:27:57 -0500 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A86FA8F25B for ; Thu, 2 Feb 2023 05:27:37 -0800 (PST) Received: by mail-wm1-x332.google.com with SMTP id m5-20020a05600c4f4500b003db03b2559eso1378852wmq.5 for ; Thu, 02 Feb 2023 05:27:37 -0800 (PST) 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=CDAYK4tECdpIzaXqNz7L+KnOqVVqDSzJi2q18roMSnE=; b=eEUi/AOZm2A5iOWJSsU34pxC+M3obxuv9FJaAvoyPpks/4gVHd4Q9lwjGz3WRlDNTb 8RgjmVa6PtkXeA/7ZzSK44gDLZbRQWAYtEjyEQUmbeAJ6tbZ23T8b5kJYhSC5F1p926J w/2GOnWHjvs4KHB6MhAMStho8FbNHcDUF6Mhtls2UFUp83/SRXDHdZUvVWgziRsW/QtZ JerIxQTM7dpjBdp+SaFIHX5PBisfGhmkUi2LfzIXaXcZLL5SXwc5ennCbUKIKPMIoKHg 24g/jmKQwBGxsB/3pWTshnyACEJRyCGa1ljMiGewqcBMZwl6cmZJtLtiB4pCpeyk03mO F34Q== 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=CDAYK4tECdpIzaXqNz7L+KnOqVVqDSzJi2q18roMSnE=; b=Kpki2O0lxnRaOZ+o4Qqb91mCPisGlaNl3W5DpTeMDhB240ERnmgFrXTTSwlRshOs72 hQtvmE4XMj2L6QK7RSI9ibIleXpXg1Fx4yu7PiP60oj1r6Hn23PgD0oOvNfoLJaqhKFP atvXCXj9zwi/b3F8nqtCPDxGjBwEIdpCjuSMEpc0tf8tCJmXdvAQkW9iynS2y/yO2dz5 2dxd30A0xoRFKWT87ezbjwdJGMHXyxYJVlh1oN79oLz7IvifYxU5MYyRuiW0LDhZ+r8Q UhrbPGKn+wlM3cyQ3xzBSqFbTxE6ycgWZohranMRWm0ADh6D0oLjwaFojMpw2SIByZMW J5TA== X-Gm-Message-State: AO0yUKXQTbwp3/sSgLzOjLx0/MvFKVsJPyhflLCELyw8+ygyLzgSQMif YtMD0j6QMWcP1wZxztUadtDQ7bM7exfMBKTf X-Google-Smtp-Source: AK7set9QeE7TwuIpSyYx3F2sVi+5J2s/ikBcI4AkdSELFeTKEO1jsliPcdPspJ3y+uoALEHveByNrA== X-Received: by 2002:a05:600c:cca:b0:3db:1919:41b5 with SMTP id fk10-20020a05600c0cca00b003db191941b5mr6041114wmb.21.1675344455764; Thu, 02 Feb 2023 05:27:35 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:34 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 6/9] for-each-repo: error on bad --config Date: Thu, 2 Feb 2023 14:27:18 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d 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. Before this, all these added tests would pass with an exit code of 0. 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 | 11 ++++++----- t/t0068-for-each-repo.sh | 6 ++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index fd0e7739e6a..224164addb3 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -32,6 +32,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; + int err; const struct option options[] = { OPT_STRING(0, "config", &config_key, N_("config"), @@ -45,11 +46,11 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) if (!config_key) die(_("missing --config=")); - /* - * Do nothing on an empty list, which is equivalent to the case - * where the config variable does not exist at all. - */ - if (repo_config_get_value_multi(the_repository, config_key, &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 3648d439a87..6b51e00da0e 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -40,4 +40,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 Thu Feb 2 13:27:19 2023 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: 13126027 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 2F50DC61DA4 for ; Thu, 2 Feb 2023 13:28:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231422AbjBBN2R (ORCPT ); Thu, 2 Feb 2023 08:28:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232297AbjBBN2J (ORCPT ); Thu, 2 Feb 2023 08:28:09 -0500 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 036648F252 for ; Thu, 2 Feb 2023 05:27:39 -0800 (PST) Received: by mail-wm1-x330.google.com with SMTP id m16-20020a05600c3b1000b003dc4050c94aso1380935wms.4 for ; Thu, 02 Feb 2023 05:27:38 -0800 (PST) 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=KxhlX7oriGNbj+RRhSBXIpRPnRGMYG9zf5rHSgVYDp4=; b=a1J+LetvFQPZxpt79v1C2qRdp/DbyG7dLJ6QFw1RTjMFpgWC3G3zzCXCfuKeoCS6YR vaJEV8fkHSieoKgvN2BaPhQ/BvgFnD5r5khNKXA61hZQc0gnfFPocV+LAeCDAyoNP3J/ Bis2BCmct2NNH35vGf8FRV8w8vcYX0xHTNZuEWZrtibfWOLa3khFdgNTZUvPjUYM6fiW 5J7z00zDoNAtzOK/Ia4LEqvFsMIy5ZHfofOLl7Me3+Nig+hZainYwSski9bfpvBhiz4s AX5liAG7nYFy/l2xvewBOcSu9+ZYfi2mlcmNPw23RpWTc9jhD6nr6bjxRiCzLdTXwr4t 1JxQ== 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=KxhlX7oriGNbj+RRhSBXIpRPnRGMYG9zf5rHSgVYDp4=; b=FlOzuLqUIStZ3rbo9JaqsajLyqMBTtF2ok8n7Dn13bOGNn1sM0gH1pKKz0HFugIS4+ SHDEELcXyDDV0SnCdRZKc+LFSahVoSakNu57UrU+Z5pPBeButOVwRuAMmBtEIw7PcZc3 cPyLiApEo6C3Jlzk/v5KSCxxDfsxBUjNDg/GdZzmg6nNbUwowc6U0IBMuxxqzNh/ljod hIZbfSJG9quv40OlAw429RhNnnSz7+z7UjyxEFpmMUYMQEHAJN4iGKXIOERX5Z6kWGsl B936eXdcvcFTVXpX9rP6lGKAUjiDQoBuMfhGlwWBCFVqYfqdYlvu9mcl1jB42vM4u3ya oPow== X-Gm-Message-State: AO0yUKXa3RjHOY6akBAGmABYBQnRv4NmkSJwJkFSvepftgJZLFpq5+Sk bs9FgMnCATL5sJCW63yDofFIjjUQeLauZrku X-Google-Smtp-Source: AK7set9NWpAYyLe1ggW336MhF/m/lIXyUrd1Mj7hIGieL7cINszJCwZwwt04AhzNlyxgkRetcRT5TA== X-Received: by 2002:a05:600c:3d8a:b0:3dd:af7a:53ed with SMTP id bi10-20020a05600c3d8a00b003ddaf7a53edmr6165529wmb.11.1675344457128; Thu, 02 Feb 2023 05:27:37 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:36 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 7/9] config API users: test for *_get_value_multi() segfaults Date: Thu, 2 Feb 2023 14:27:19 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As we'll discuss in the subsequent commit these tests all show *_get_value_multi() API users unable to handle there being a value-less key in the config, which is represented with a "NULL" for that entry in the "string" member of the returned "struct string_list", causing a segfault. These added tests exhaustively test for that issue, as we'll see in a subsequent commit we'll need to change all of the API users of *_get_value_multi(). These cases were discovered by triggering each one individually, and then adding these tests. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t4202-log.sh | 11 +++++++++++ t/t5310-pack-bitmaps.sh | 16 ++++++++++++++++ t/t7004-tag.sh | 12 ++++++++++++ t/t7413-submodule-is-active.sh | 12 ++++++++++++ t/t7900-maintenance.sh | 23 +++++++++++++++++++++++ 5 files changed, 74 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 2ce2b41174d..e4f02d8208b 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -835,6 +835,17 @@ test_expect_success 'log.decorate configuration' ' ' +test_expect_failure '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 + git log --decorate=short +' + 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 7d8dee41b0d..0306b399188 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -404,6 +404,22 @@ test_bitmap_cases () { ) ' + test_expect_failure '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 + git repack -adb + ) + ' + 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..f343551a7d4 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1843,6 +1843,18 @@ test_expect_success 'invalid sort parameter in configuratoin' ' test_must_fail git tag -l "foo*" ' +test_expect_failure '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 + git tag -l --sort=version:refname +' + 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..bfe27e50732 100755 --- a/t/t7413-submodule-is-active.sh +++ b/t/t7413-submodule-is-active.sh @@ -51,6 +51,18 @@ test_expect_success 'is-active works with submodule..active config' ' test-tool -C super submodule is-active sub1 ' +test_expect_failure '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 + + test-tool -C super submodule is-active sub1 +' + 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 823331e44a0..d82eac6a471 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -524,6 +524,29 @@ test_expect_success 'register and unregister' ' git maintenance unregister --config-file ./other --force ' +test_expect_failure '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 + git maintenance register +' + +test_expect_failure '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 + git maintenance unregister && + git maintenance unregister --force +' + test_expect_success !MINGW 'register and unregister with regex metacharacters' ' META="a+b*c" && git init "$META" && From patchwork Thu Feb 2 13:27:20 2023 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: 13126029 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 B1801C05027 for ; Thu, 2 Feb 2023 13:28:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232051AbjBBN2V (ORCPT ); Thu, 2 Feb 2023 08:28:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232462AbjBBN2K (ORCPT ); Thu, 2 Feb 2023 08:28:10 -0500 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2C9C48F257 for ; Thu, 2 Feb 2023 05:27:40 -0800 (PST) Received: by mail-wm1-x32d.google.com with SMTP id n28-20020a05600c3b9c00b003ddca7a2bcbso1386457wms.3 for ; Thu, 02 Feb 2023 05:27:40 -0800 (PST) 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=JyReybXfSSS5GObpgCSDEFEABdI7t3qXkrG31jBibps=; b=S//cwNujOj9Gc+RI1kFHnrG5JapIjiONrPbdY4iH65kONQpdTdrL0MzEstdTctr3cO 8Mdk4fCjmIA5lZkyKUXC9GNDxF4Ayu83unJzwnfM8QijE/gCpOGoMHgamfYDNHoAGoBN W60fa32HMf85abKvrfMI31pQs95MFXitg9oKoV5iZ40sCY/UDlRtgdACApjJy3TWjzhk ROj6mrNQYQU05qOCLg2SA/iigajIWqsN3ZYQ2Nfrjw5WR24bU1mkkR03Q5r1zDcvM6BR jFwjFPZkSSm9QxJifQopqJ7JvTEFzGyvoLLrc+pjPIuJYf+Ng8VtI4TPj+T+RaudocTb 5bDw== 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=JyReybXfSSS5GObpgCSDEFEABdI7t3qXkrG31jBibps=; b=Pq2b8loA3Yp9FdrGz9CNGsrvCXc880VFxkZjZs2Mw7oyQdGdENHIT5xBszS3a95JZU wpwZ8VekL7qdN6igvrst2a/ZrocJ7vYYjq3EEVk1KulrrPQT/yH0ukA5j6SAuB2uk6J5 9oXV6D005MaNPalAFGCp6SMS0ICWacKpolTtlWER/Lf0Nc0Ta1T1x+JEpqAyozy42N5p OlHzh+/XIxV8YYAP0l3nwR5SfKLskcPFMWyUmC/QNTchkrLbttOPPLBwPzzR6dRE6XIt +WYBh8hV2SdTJXgtcHNeR5weFjxFAKVpcB4ncSCd75HJ45xdJuvsjFHCf++14WmET/p6 0ECw== X-Gm-Message-State: AO0yUKXAdvJpDLzwMZBNxg3pTa3f4xw2XAK0PtD3lAvruoB2ItQG3Ix/ EKM/jIACLYEE8iLhk2EF6iN6bSkMWEkFNMe7 X-Google-Smtp-Source: AK7set/r8TvyRejvQ+MbSxp4XbBKY2Dru01Yzvyhgfh8m0tnUXF11Ibf9iYx+yHFClZtAmfYuPufbA== X-Received: by 2002:a05:600c:3d1a:b0:3db:2858:db84 with SMTP id bh26-20020a05600c3d1a00b003db2858db84mr5852735wmb.34.1675344458283; Thu, 02 Feb 2023 05:27:38 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:37 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 8/9] config API: add "string" version of *_value_multi(), fix segfaults Date: Thu, 2 Feb 2023 14:27:20 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d 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, most users 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 "*_get_string_multi()" variant of the low-level "_*value_multi()" 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 three remaining files using the low-level API: - Two cases in "builtin/submodule--helper.c", where it's used safely to see if any config exists. We could refactor these away from "multi" to some "does it exist?" function, as [4] did, but as that's orthogonal to the "string" safety we're introducing here let's leave them for now. - One in "builtin/for-each-repo.c", which we'll convert in a subsequent commit. - The "t/helper/test-config.c" code added in [4]. As seen in the preceding commit we need to give the "t/helper/test-config.c" caller these "NULL" entries. We could also alter the underlying git_configset_get_value_multi() function to be "string safe", but doing so would leave no room for other variants of "*_get_value_multi()" that coerce to other types. Such coercion can't be built on the string version, since as we've established "NULL" is a true value in the boolean context, but if we coerced it to "" for use in a list of strings it'll be subsequently coerced to "false" as a boolean. 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. https://lore.kernel.org/git/patch-07.10-c01f7d85c94-20221026T151328Z-avarab@gmail.com/ 4. 4c715ebb96a (test-config: add tests for the config_set API, 2014-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 6 +++--- builtin/log.c | 4 ++-- config.c | 32 ++++++++++++++++++++++++++++++++ config.h | 19 +++++++++++++++++++ pack-bitmap.c | 2 +- submodule.c | 2 +- t/t4202-log.sh | 8 ++++++-- t/t5310-pack-bitmaps.sh | 8 ++++++-- t/t7004-tag.sh | 9 +++++++-- t/t7413-submodule-is-active.sh | 8 ++++++-- t/t7900-maintenance.sh | 25 ++++++++++++++++++++----- versioncmp.c | 4 ++-- 12 files changed, 105 insertions(+), 22 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 2b3da377d52..9497bdf23e4 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1510,7 +1510,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) if (git_config_get("maintenance.strategy")) git_config_set("maintenance.strategy", "incremental"); - if (!git_config_get_value_multi(key, &list)) { + if (!git_config_get_string_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1578,8 +1578,8 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi git_configset_add_file(&cs, config_file); } if (!(config_file - ? git_configset_get_value_multi(&cs, key, &list) - : git_config_get_value_multi(key, &list))) { + ? git_configset_get_string_multi(&cs, key, &list) + : git_config_get_string_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 cec8cabd21e..481685d5263 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -184,8 +184,8 @@ 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_value_multi("log.excludeDecoration", - &config_exclude)) { + if (!git_config_get_string_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/config.c b/config.c index ce5d50a490c..30867663997 100644 --- a/config.c +++ b/config.c @@ -2447,6 +2447,25 @@ int git_configset_get_value_multi(struct config_set *cs, const char *key, 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_string_multi(struct config_set *cs, const char *key, + const struct string_list **dest) +{ + int ret; + + if ((ret = git_configset_get_value_multi(cs, key, dest))) + return ret; + if ((ret = for_each_string_list((struct string_list *)*dest, + check_multi_string, (void *)key))) + return ret; + + return 0; +} + int git_configset_get(struct config_set *cs, const char *key) { struct config_set_element *e; @@ -2615,6 +2634,13 @@ int repo_config_get_value_multi(struct repository *repo, const char *key, return git_configset_get_value_multi(repo->config, key, dest); } +int repo_config_get_string_multi(struct repository *repo, const char *key, + const struct string_list **dest) +{ + git_config_check_init(repo); + return git_configset_get_string_multi(repo->config, key, dest); +} + int repo_config_get_string(struct repository *repo, const char *key, char **dest) { @@ -2730,6 +2756,12 @@ 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_string_multi(const char *key, + const struct string_list **dest) +{ + return repo_config_get_string_multi(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 fbc153cdc96..d98a06352e3 100644 --- a/config.h +++ b/config.h @@ -472,6 +472,19 @@ RESULT_MUST_BE_USED int git_configset_get_value_multi(struct config_set *cs, const char *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_string_multi(struct config_set *cs, const char *key, + const struct string_list **dest); + /** * Clears `config_set` structure, removes all saved variable-value pairs. */ @@ -518,6 +531,9 @@ int repo_config_get_value(struct repository *repo, 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_string_multi(struct repository *repo, const char *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, @@ -578,6 +594,9 @@ int git_config_get_value(const char *key, const char **value); RESULT_MUST_BE_USED int git_config_get_value_multi(const char *key, const struct string_list **dest); +RESULT_MUST_BE_USED +int git_config_get_string_multi(const char *key, + const struct string_list **dest); /** * Resets and invalidates the config cache. diff --git a/pack-bitmap.c b/pack-bitmap.c index 15c5eb507c0..d003c7e60b4 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2316,7 +2316,7 @@ const struct string_list *bitmap_preferred_tips(struct repository *r) { const struct string_list *dest; - if (!repo_config_get_value_multi(r, "pack.preferbitmaptips", &dest)) + if (!repo_config_get_string_multi(r, "pack.preferbitmaptips", &dest)) return dest; return NULL; } diff --git a/submodule.c b/submodule.c index 4b6f5223b0c..30a103246ec 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_value_multi(repo, "submodule.active", &sl)) { + if (!repo_config_get_string_multi(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 e4f02d8208b..ae73aef922f 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -835,7 +835,7 @@ test_expect_success 'log.decorate configuration' ' ' -test_expect_failure 'parse log.excludeDecoration with no value' ' +test_expect_success 'parse log.excludeDecoration with no value' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -843,7 +843,11 @@ test_expect_failure 'parse log.excludeDecoration with no value' ' [log] excludeDecoration EOF - git log --decorate=short + 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' ' diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 0306b399188..526a5a506eb 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -404,7 +404,7 @@ test_bitmap_cases () { ) ' - test_expect_failure 'pack.preferBitmapTips' ' + test_expect_success 'pack.preferBitmapTips' ' git init repo && test_when_finished "rm -rf repo" && ( @@ -416,7 +416,11 @@ test_bitmap_cases () { [pack] preferBitmapTips EOF - git repack -adb + cat >expect <<-\EOF && + error: missing value for '\''pack.preferbitmaptips'\'' + EOF + git repack -adb 2>actual && + test_cmp expect actual ) ' diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index f343551a7d4..f4a31ada79a 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1843,7 +1843,7 @@ test_expect_success 'invalid sort parameter in configuratoin' ' test_must_fail git tag -l "foo*" ' -test_expect_failure 'version sort handles empty value for versionsort.{prereleaseSuffix,suffix}' ' +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 && @@ -1852,7 +1852,12 @@ test_expect_failure 'version sort handles empty value for versionsort.{prereleas prereleaseSuffix suffix EOF - git tag -l --sort=version:refname + 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' ' diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh index bfe27e50732..887d181b72e 100755 --- a/t/t7413-submodule-is-active.sh +++ b/t/t7413-submodule-is-active.sh @@ -51,7 +51,7 @@ test_expect_success 'is-active works with submodule..active config' ' test-tool -C super submodule is-active sub1 ' -test_expect_failure 'is-active handles submodule.active config missing a value' ' +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 && @@ -60,7 +60,11 @@ test_expect_failure 'is-active handles submodule.active config missing a value' active EOF - test-tool -C super submodule is-active sub1 + 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' ' diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index d82eac6a471..487e326b3fa 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -524,7 +524,7 @@ test_expect_success 'register and unregister' ' git maintenance unregister --config-file ./other --force ' -test_expect_failure 'register with no value for maintenance.repo' ' +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 && @@ -532,10 +532,15 @@ test_expect_failure 'register with no value for maintenance.repo' ' [maintenance] repo EOF - git maintenance register + 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_failure 'unregister with no value for 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 && @@ -543,8 +548,18 @@ test_expect_failure 'unregister with no value for maintenance.repo' ' [maintenance] repo EOF - git maintenance unregister && - git maintenance unregister --force + 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' ' diff --git a/versioncmp.c b/versioncmp.c index 60c3a517122..7498da96e0e 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -164,8 +164,8 @@ int versioncmp(const char *s1, const char *s2) const char *const oldk = "versionsort.prereleasesuffix"; const struct string_list *newl; const struct string_list *oldl; - int new = git_config_get_value_multi(newk, &newl); - int old = git_config_get_value_multi(oldk, &oldl); + int new = git_config_get_string_multi(newk, &newl); + int old = git_config_get_string_multi(oldk, &oldl); if (!new && !old) warning("ignoring %s because %s is set", oldk, newk); From patchwork Thu Feb 2 13:27:21 2023 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: 13126028 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 BA67DC636D4 for ; Thu, 2 Feb 2023 13:28:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232645AbjBBN2U (ORCPT ); Thu, 2 Feb 2023 08:28:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58502 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232452AbjBBN2J (ORCPT ); Thu, 2 Feb 2023 08:28:09 -0500 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 57E4E8C1F6 for ; Thu, 2 Feb 2023 05:27:41 -0800 (PST) Received: by mail-wm1-x336.google.com with SMTP id m16-20020a05600c3b1000b003dc4050c94aso1381001wms.4 for ; Thu, 02 Feb 2023 05:27:41 -0800 (PST) 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=mdDbSHrPhjpMZN7ZFrZsb78p+pxp/qdpL2PZ2jCYbZw=; b=Af0x4+5CAoTSb+Rm15vo3sPEWNguigdETNgfpjoXTWANx8OEnziXv3SzUa72NwrSzi NcvhMwOzgTzKCSyN/HpIX728cmaX5z7tz9NkFQmGfQ/BXBUYKaHG/mrOud6bUsmKKkFR S2oXsdEtUsA+UBdkGxbbBa/hENSnpvI/zF+tHd8VEi5zXoE6eKlql7ejWsdOy2W+nRCy O5k2Ka3uv1B7uhjGEfhRMSAkOS67OyANkb5wP5kJoXRL3BcI1va4pLnJ7msbzw0PcJQr aqfj3W0XfbdLZj5jqhhWUzXoGlaI7mXg7sqv97XwJyKgUFZ9+Zb8qRGk66YURZRJ1XW9 5sTg== 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=mdDbSHrPhjpMZN7ZFrZsb78p+pxp/qdpL2PZ2jCYbZw=; b=qzvfZsSYlfctogII//3H9Bxmo15pLE/j/TLcmbfnmm6Zv0noVqiUD8bhaa8Sic0MVz 7wxNaZ2kcxHncnXViJ5iZVOkj2tszg8V8QbgWZQ+nmq/KAcGe6YDmNe19Md1xPhJx488 wpdLIGgOGjwJ7SeTnXpSd31+aCwInqSv7PsELZKYNmchLgrQcy2ZRdhjmBbgF0EVGhCG LBjRr/UBYwgJ0o/OiIWLk3msZsjTKza4QBeBti8qxDikErXUJZ6bRfUQP2qclGzpruXR ZnwvXodn0YS7CZCpW8S3HdQyqkBEEAc1AMzXRzUk3njhqCuF43PGFfE2fWeVdvWJJdzf NY6g== X-Gm-Message-State: AO0yUKXOvGXmc9vx6pvF/YElo2+9g2rkrwCPnRB9rwQhOZn0wsNRW1LN O9P5NwlCeJsa/Nke/WzCXv41lbZahWyx4TZA X-Google-Smtp-Source: AK7set9/UzUs7Wqr7vCiVSejZTjWnCwAeqk2i/3LoVR7MzdreNOmDyLM1OGJpsgc23V5/UGIlRf9Rg== X-Received: by 2002:a05:600c:cca:b0:3db:bc5:b2ae with SMTP id fk10-20020a05600c0cca00b003db0bc5b2aemr5472403wmb.41.1675344459474; Thu, 02 Feb 2023 05:27:39 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z18-20020a1c4c12000000b003dc48a2f997sm4306052wmf.17.2023.02.02.05.27.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Feb 2023 05:27:38 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Elijah Newren , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_?= =?utf-8?q?G=C3=A1bor?= , Glen Choo , Calvin Wan , Emily Shaffer , raymond@heliax.dev, zweiss@equinix.com, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v4 9/9] for-each-repo: with bad config, don't conflate and Date: Thu, 2 Feb 2023 14:27:21 +0100 Message-Id: X-Mailer: git-send-email 2.39.1.1397.g8c8c074958d 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 "*_string_multi()" 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 224164addb3..ce8f7a99086 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -46,7 +46,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_string_multi(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 6b51e00da0e..4b90b74d5d5 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -46,4 +46,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