From patchwork Tue Mar 28 14:04: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: 13191125 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 5127DC6FD18 for ; Tue, 28 Mar 2023 14:06:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231251AbjC1OGS (ORCPT ); Tue, 28 Mar 2023 10:06:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47970 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231757AbjC1OGP (ORCPT ); Tue, 28 Mar 2023 10:06:15 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 52C4BCC0D for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id l15-20020a05600c4f0f00b003ef6d684102so4194461wmq.3 for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012277; 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=1OOK10i7DgT28p7HLiKJC10k2AsXfqCe8TFpdOiB4yc=; b=i0VdRXk9nnBeMOSa4ym4tYaZQ0cwVMVrcrHPy6hbBUSV2e3dcs3b7aKLnC3hCcQ5gV jmhGn1jEud2Mp6pYcES9sLqrqlqvlshaylTZJwH6OZabyaoNjfpLG7efzJHDZicZfFxQ SXwU19rYjq7sj7F7YtDG7Mpt8RQ3/eoWJqLEOsx/SwKOmkQhyaUeof6n5zNEb225T000 F/+yfmm2uwPm7ylbNsXjxOsApshpavvyd0SYpZcooZjvjlaiJrlCOs1Bn1o9vtEe9kZl Tre+hTcKtFNTGna1BKTXqSZU/7ZQ5XlzFGqdC2FTT1DQUOXbN6E57t4onrhpcsHQ1Hbn +Vig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012277; 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=1OOK10i7DgT28p7HLiKJC10k2AsXfqCe8TFpdOiB4yc=; b=ch3mZ7WRAfpeCqfFQiTfbJTbxMCSFLbfVdcVWqtKdCtjaJisBCocyyZoJn8ehIv+UI 9SPoUHmfeHlUkDrpL9yj/5rC0KfsPIi/DcxRlH72wQkqJ0gYorPn4s44sxS6yLeGc+5P xvgrJuw0HQPV7QZ0vJGFk+pARbdVs9eM5KflLfaCy/8ypG58ggX/bBBPskULQYFOH3nX t/jWSW0yKSZn+IXwNgspk1I4xOesd3fDQg2jP2wfReWcpjK+WNF3GsqDsuHDH+/IJaEX uOoX9N8ugkzeiL+Z6FzEziG/5U6zjaUuNfqB53LcqZfPhjtPMpG5CLJxm6xxPnryiltG KipA== X-Gm-Message-State: AO0yUKVak5Y5MdK6Z19wxdqHkYZb3cArNvMn76NV7aeYoJ2lWZp0G58u bHJ+YmsJrO6XfmRDDyb6I6OqdZ5bm+b6zg== X-Google-Smtp-Source: AK7set9A8MbuV9cM6zL3IHYk1sUAzOu5KAuJpRBTWk6u5+yILwPacIkGybl66xHcXKLd8OShG7MAfw== X-Received: by 2002:a1c:4b06:0:b0:3ed:316d:668d with SMTP id y6-20020a1c4b06000000b003ed316d668dmr12544104wma.5.1680012277213; Tue, 28 Mar 2023 07:04:37 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:36 -0700 (PDT) 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 v8 1/9] config tests: cover blind spots in git_die_config() tests Date: Tue, 28 Mar 2023 16:04:20 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 Tue Mar 28 14:04: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: 13191126 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 0C5ECC76196 for ; Tue, 28 Mar 2023 14:06:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232202AbjC1OGT (ORCPT ); Tue, 28 Mar 2023 10:06:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231890AbjC1OGQ (ORCPT ); Tue, 28 Mar 2023 10:06:16 -0400 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 70B5AC179 for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id l15-20020a05600c4f0f00b003ef6d684102so4194508wmq.3 for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012278; 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=4vhB/VvuxpEFLBgJp0BccsdhTjWfa6KSYpVrKo2IRgo=; b=fPrL60inMRKj9Z0sovxuWaygRd3u3b+sgVSTOXhB7EPzXNBWrGyJ7ON1HgrkgDRb3f 6Si9tJCIQdrrVrg1SXNxYtSg6V0dayNQAUHbMbGICYvPY1lPeMFfpO9uSoHSjcDULnmZ di58Bj2nlMPWJ6S5jdu2NiwjuEYkx+/2VFwv4xBfkQ4f6JhrYKe31KarOWALVJ3md1rI KHz5QrWRDZwSmK35VWDX7DOMhtc7Ir8xjPzJw9f1Z49JEtmoF4NukciX7ZvFZxytFge8 0fw4Xd2X5tyTly/jtpM9RnH56RueLWLuHbxMhTJbtMPyyCVCXVWZ63lye6cQKHEjCRrH e43g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012278; 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=4vhB/VvuxpEFLBgJp0BccsdhTjWfa6KSYpVrKo2IRgo=; b=DqtTUfkxqVzoKCjuWH98JiuRz6ksI3OAzr850Xr2B77q4aBnvJZLMJZimTd+IZkce5 eufLuv2hxJpGDlvpNmU9zr47vUuRmOJv06TtCZn9dlEmPfItUtxKWFhoSv/38LFlcvdo YFg2Inrb2IBHxZAM6NCzARE9cnus1Od4i0B3OwmUjOsFPrVXiTTXeqkf79G5QFE+ZvSx vxx7HI4Aqv2YsohXaQXzkaFkc1c0KhHQUttQj35wMntJh/NRjbtg4GCzONUtwVsQ3wvm CzNT9SbxmMwn6iCkC0cqI7JgI1x7jHPimbcuoQcfqKxqc6jT43um0C4kEj2tkvI1VRjp SfFg== X-Gm-Message-State: AO0yUKVZ6NY+3+olEYDq0sQ/wI5d4+FBTA2ZPrryBACcJ1M4bna7jNG6 C9XQcHB3DsQ0+LjxvnurMeijC0+zoIjLXg== X-Google-Smtp-Source: AK7set/NtqIRxTMq72QrCQ/eHjseET4A11QZlarpGMcEPw4VJj1W3710XQ37EZx7BckerGkjBUgd+Q== X-Received: by 2002:a1c:7702:0:b0:3ed:4f7d:f6ee with SMTP id t2-20020a1c7702000000b003ed4f7df6eemr12399504wmi.14.1680012278343; Tue, 28 Mar 2023 07:04:38 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:37 -0700 (PDT) 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 v8 2/9] config tests: add "NULL" tests for *_get_value_multi() Date: Tue, 28 Mar 2023 16:04:21 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 | 65 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index b38e158d3b2..4be1ab1147c 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -146,6 +146,71 @@ 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 && + + # Value-less in the middle of a list + 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 && + + # Value-less at the end of a least + cat >"$config" <<-\EOF && + [a]key=x + [a]key=y + [a]key + EOF + case "$op" in + *_multi) + cat >expect <<-\EOF + x + y + (NULL) + EOF + ;; + *) + cat >expect <<-\EOF + (NULL) + 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 Tue Mar 28 14:04:22 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: 13191127 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 EB8B2C6FD18 for ; Tue, 28 Mar 2023 14:06:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232277AbjC1OGW (ORCPT ); Tue, 28 Mar 2023 10:06:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48002 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232144AbjC1OGT (ORCPT ); Tue, 28 Mar 2023 10:06:19 -0400 Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4825CC3E for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) Received: by mail-wm1-x335.google.com with SMTP id t17-20020a05600c451100b003edc906aeeaso827554wmo.1 for ; Tue, 28 Mar 2023 07:04:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012279; 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=HEGOgRkYR1DOCIDyuWyGePoxaagPZ8NGdB/VQtqJ7y8=; b=HdJ/B0pw7xiE/99N+2jwFxR9PIb3uNru45jeuZAHU2e+5RZOMM/v+glEfwPxNOU080 PkNiIbID3T8AH1hPEgAwsCSf8/53GMvEJe3sWds9cm1nTk1/hZNk88FcQc9wJRNIi7x0 qbeBcO0lhDw0OaFhJ/HsYCh+SDVVU8XxmKCk+l92O55jouzWU9TW3qTSnnTUMZ9/J4Am dDCc/pdaCJDf0Mb//3nwcjVj9mqT+nOvzIhjhh7jyT5wZC1lwZU/Sj46CBVtCf8kp1RM ewElBmkwRk9ZGzVrF8BN4nUb0CCLMmGrgUPAx4SL8FhdrAi7CyLeyuoyDCw+6gIo4OHh keXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012279; 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=HEGOgRkYR1DOCIDyuWyGePoxaagPZ8NGdB/VQtqJ7y8=; b=xC024mB9Ip3eeCJgF8pUsDTp+/ZZf1M2NzgLHGTRwPgs1P4Q2Fhuut14OIlKLts6nO rqmVEgegU/EsX7KYiiwxuBbWoUepaAEX7cZibmCEkfHKxN0C03Ze0hbTZfqf/evR4nAM ua/0W00bJrUGpzMAN3bFWKKUNbfBNlhf/IpUjKwyJHQBgdCFNgiWPz+2RQiCt0QTiUmC NH4PyZuSphuBi//S0n1UxwZRrhIZOGDLjWGxLntm6KyyvgaKH73q3MKxx9poKE/PKKhA d8WUnmzooC/Erk/GIpdhqG9yweFqbeyzjC7KhtlcGWMiFn9yfpC7/PsdKQLIEQtK230H rteg== X-Gm-Message-State: AO0yUKXAoxSsfcfGz2N0YriQ+jyNvTReLSMCAUYIAk/TnpLWaxwdd1oi g51Po0K6endSImzaZvox/xJNyJ5VxDRxrg== X-Google-Smtp-Source: AK7set+etKL29zHJsabJqE3MJ9uxBqb0XfkUWNKGJoLeFEB3MnYRyYGnD8FFRuZGrIBPNHsg+Eyj6g== X-Received: by 2002:a05:600c:45cd:b0:3e1:e149:b67b with SMTP id s13-20020a05600c45cd00b003e1e149b67bmr15406906wmo.18.1680012279377; Tue, 28 Mar 2023 07:04:39 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:38 -0700 (PDT) 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 v8 3/9] config API: add and use a "git_config_get()" family of functions Date: Tue, 28 Mar 2023 16:04:22 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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() (and then git_config_get_value() etc.) 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. A "NULL as optional dest" pattern is also more fragile, as the intent of the caller might be misinterpreted if he were to accidentally pass "NULL", e.g. when "dest" is passed in from another function. 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. This still leaves various inconsistencies and clobbering or ignoring of the return value in place. E.g here we're modifying configset_add_value(), but ever since it was added in [2] we've been ignoring its "int" return value, but as we're changing the configset_find_element() it uses, let's have it faithfully ferry that "ret" along. Let's also use the "RESULT_MUST_BE_USED" macro introduced in [3] to assert that we're checking the return value of configset_find_element(). We're leaving the same change to configset_add_value() for some future series. Once we start paying attention to its return value we'd need to ferry it up as deep as do_config_from(), and would need to make least read_{,very_}early_config() and git_protected_config() return an "int" instead of "void". Let's leave that for now, and focus on the *_get_*() functions. 1. 3c8687a73ee (add `config_set` API for caching config-like files, 2014-07-28) 2. https://lore.kernel.org/git/xmqqczadkq9f.fsf@gitster.g/ 3. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01), Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/gc.c | 5 +--- builtin/submodule--helper.c | 7 +++-- builtin/worktree.c | 3 +-- config.c | 51 ++++++++++++++++++++++++++++++++----- config.h | 22 ++++++++++++++++ t/helper/test-config.c | 22 ++++++++++++++++ t/t1308-config-set.sh | 43 ++++++++++++++++++++++++++++++- 7 files changed, 135 insertions(+), 18 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c58fe8c936c..b7251840e20 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1494,7 +1494,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; @@ -1509,9 +1508,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 d05d1a84623..647dbb932bc 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -559,7 +559,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; @@ -2745,7 +2745,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; @@ -3142,7 +3142,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; @@ -3187,7 +3186,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 80d05e246d8..476325ef98f 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -320,7 +320,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) || @@ -339,7 +338,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 d0aff55fa66..ba2ec3b54ee 100644 --- a/config.c +++ b/config.c @@ -2292,23 +2292,29 @@ 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) +RESULT_MUST_BE_USED +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) @@ -2317,8 +2323,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. @@ -2428,8 +2437,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) @@ -2568,6 +2594,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) { @@ -2682,6 +2714,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 7606246531a..72d83e21e3d 100644 --- a/config.h +++ b/config.h @@ -465,6 +465,13 @@ void git_configset_clear(struct config_set *cs); * value in the 'dest' pointer. */ +/** + * git_configset_get() returns negative values on error, see + * repo_config_get() below. + */ +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. @@ -485,6 +492,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, @@ -521,8 +536,15 @@ 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. */ +RESULT_MUST_BE_USED +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 diff --git a/t/helper/test-config.c b/t/helper/test-config.c index 4ba9eb65606..cbb33ae1fff 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -14,6 +14,8 @@ * get_value_multi -> prints all values for the entered key in increasing order * of priority * + * get -> print return value for the entered key + * * get_int -> print integer value for the entered key or die * * get_bool -> print bool value for the entered key or die @@ -109,6 +111,26 @@ int cmd__config(int argc, const char **argv) printf("Value not found for \"%s\"\n", argv[2]); goto exit1; } + } else if (argc == 3 && !strcmp(argv[1], "get")) { + int ret; + + if (!(ret = git_config_get(argv[2]))) + goto exit0; + else if (ret == 1) + printf("Value not found for \"%s\"\n", argv[2]); + else if (ret == -CONFIG_INVALID_KEY) + printf("Key \"%s\" is invalid\n", argv[2]); + else if (ret == -CONFIG_NO_SECTION_OR_NAME) + printf("Key \"%s\" has no section\n", argv[2]); + else + /* + * A normal caller should just check "ret < + * 0", but for our own tests let's BUG() if + * our whitelist of git_config_parse_key() + * return values isn't exhaustive. + */ + BUG("Key \"%s\" has unknown return %d", argv[2], ret); + goto exit1; } else if (argc == 3 && !strcmp(argv[1], "get_int")) { if (!git_config_get_int(argv[2], &val)) { printf("%d\n", val); diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 4be1ab1147c..7def7053e1c 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -58,6 +58,8 @@ test_expect_success 'setup default config' ' skin = false nose = 1 horns + [value] + less EOF ' @@ -116,6 +118,45 @@ test_expect_success 'find value with the highest priority' ' check_config get_value case.baz "hask" ' +test_expect_success 'return value for an existing key' ' + test-tool config get lamb.chop >out 2>err && + test_must_be_empty out && + test_must_be_empty err +' + +test_expect_success 'return value for value-less key' ' + test-tool config get value.less >out 2>err && + test_must_be_empty out && + test_must_be_empty err +' + +test_expect_success 'return value for a missing key' ' + cat >expect <<-\EOF && + Value not found for "missing.key" + EOF + test_expect_code 1 test-tool config get missing.key >actual 2>err && + test_cmp actual expect && + test_must_be_empty err +' + +test_expect_success 'return value for a bad key: CONFIG_INVALID_KEY' ' + cat >expect <<-\EOF && + Key "fails.iskeychar.-" is invalid + EOF + test_expect_code 1 test-tool config get fails.iskeychar.- >actual 2>err && + test_cmp actual expect && + test_must_be_empty out +' + +test_expect_success 'return value for a bad key: CONFIG_NO_SECTION_OR_NAME' ' + cat >expect <<-\EOF && + Key "keynosection" has no section + EOF + test_expect_code 1 test-tool config get keynosection >actual 2>err && + test_cmp actual expect && + test_must_be_empty out +' + test_expect_success 'find integer value for a key' ' check_config get_int lamb.chop 65 ' @@ -272,7 +313,7 @@ test_expect_success 'proper error on error in default config files' ' cp .git/config .git/config.old && test_when_finished "mv .git/config.old .git/config" && echo "[" >>.git/config && - echo "fatal: bad config line 34 in file .git/config" >expect && + echo "fatal: bad config line 36 in file .git/config" >expect && test_expect_code 128 test-tool config get_value foo.bar 2>actual && test_cmp expect actual ' From patchwork Tue Mar 28 14:04:23 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: 13191128 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 56584C76196 for ; Tue, 28 Mar 2023 14:06:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232507AbjC1OG0 (ORCPT ); Tue, 28 Mar 2023 10:06:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231890AbjC1OGV (ORCPT ); Tue, 28 Mar 2023 10:06:21 -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 8766818B for ; Tue, 28 Mar 2023 07:04:58 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id l15-20020a05600c4f0f00b003ef6d684102so4194559wmq.3 for ; Tue, 28 Mar 2023 07:04:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012280; 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=8pmRz3v3Hrht8hHKGL7OOBR85+GFIGz7ixvjtemVP/Y=; b=MXPuzCg+yZW4dtFdXpdhyIXKCqUPeU/d+DTdzHbQRcF8I0YzgOA88BE00pA+wlnZbz FUOFBADwSH7k8XXLlwGpZ1XkM143dg0VGKDFPW8bKich8Z6/GDN6OmuD8MDRjFlv6jPD RFrg2K4I9PTB8veS/Dd4bvcomPP6cvdb7tSsrQPTIH6xWv0Jq6Bp+hyBGUq2NSzfme4u 7CkYpASTOkkgKqUrbdxluUrWMVdlrFjQP0BLoS9Ft4jS8kSdxVofJUzJLsfLmoi9UUKx JGDrzwBOLPrSPZi2PJbr7KYpdbLlnkySjsqRzULqryH7mQRvK+zdcg0ZR1i6GxmQh8q/ /tQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012280; 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=8pmRz3v3Hrht8hHKGL7OOBR85+GFIGz7ixvjtemVP/Y=; b=GAHXMAa+1TjFDr26TWpPlzRN6jW+40OH8LArgz47OVvJwH7pkMiYacXvuKisHpS8vB DQDcwdQ38yP3bTH5QbxIEw/AJxE76AOqmjF7g9sNNBxPQbegPl0OE60THmAlpzdAK6MH bVJL0eKQp0e1AK40HZdRWzBzHRcO0SJrjw0Z274SUCb+vZgrR71HqeeVMDR4V96QLQUA 61vLBEJWy6wu4+pnTq0coa9KoL5CyUzSmAfDPiDXTYBodyx62Ro7yGcGAX27aAVA6wu2 X8vXwEOglUSXCoN1LcE4ZMxmbzy40CZVFWHsK+ob52jp73Mr7KQpoQTEjj2SdqtVc5ED N7Kg== X-Gm-Message-State: AO0yUKXMx5A23QRTH0EY45OdWb1iCLb8gOvWbVH2yJl/H0Yx3DnLxLNX Jsv4/ll3yVBN9xIFOv7rfIkA2OH7c3F08g== X-Google-Smtp-Source: AK7set8NUTcGSSN+huGbJWIvF/ahO6DpYHt+3uzQy326NHOhlYFE9hschThkiwas4NPrfdN8l+dq4A== X-Received: by 2002:a7b:cb44:0:b0:3ee:672d:caa3 with SMTP id v4-20020a7bcb44000000b003ee672dcaa3mr13377667wmj.18.1680012280165; Tue, 28 Mar 2023 07:04:40 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:39 -0700 (PDT) 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 v8 4/9] versioncmp.c: refactor config reading next commit Date: Tue, 28 Mar 2023 16:04:23 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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. Renaming "deprecated_prereleases" to "oldl" doesn't help us to avoid line wrapping now, but it will in a subsequent commit. Let's also split out the names of the config variables into variables of our own, 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 Tue Mar 28 14:04:24 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: 13191129 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 A9CC3C76196 for ; Tue, 28 Mar 2023 14:06:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232588AbjC1OGh (ORCPT ); Tue, 28 Mar 2023 10:06:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48024 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232211AbjC1OGV (ORCPT ); Tue, 28 Mar 2023 10:06:21 -0400 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE5BD977F for ; Tue, 28 Mar 2023 07:04:58 -0700 (PDT) Received: by mail-wm1-x32f.google.com with SMTP id o24-20020a05600c511800b003ef59905f26so7518514wms.2 for ; Tue, 28 Mar 2023 07:04:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012281; 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=KeIqCCu9FZo6In2P/ZtaoSAtYgjuEnmbO7UP4EcZLiU=; b=VJZatzh3HC3BTWyU4zgNbxZFDAJN9ButzaeO+/TUnPl6f10J9yOyAx0/oe6QBk/w0M Qb6YTCaW1d6ygRThAwlSwvq5O9SLUcmR12U36ayafavCZlEcytM5RhDFBsGTsD/1jrvK NwTNNF2Yb+VpkbZuyjLYQUw832uInlI1smW3zlmdzAWt8IpmV5er8TIyM8d3u7AXtizw G2DtcyDoJjzgzmXI4ByeUxETyJLbBTu8mfz20raOTTnOGAG0wl3XOrAXa/lR00Q3czsw EMlLYbNRFB5EQn3JPPTzKU/O7Y/R5RYWGlZOnXhp+M5CSzM150t0kFEXX90kHww7JwpD K/Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012281; 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=KeIqCCu9FZo6In2P/ZtaoSAtYgjuEnmbO7UP4EcZLiU=; b=4MUoS50+exKBSc8CldpLmEEKWvYOeLUfjC5SqD2zqy/3IbyXGizoi4A4nApMGT0R1f SVEgHEwLyUG5RsJHN8scIXE5WTETnlLOmfA1eu/t2hABuP/QJrBfnXl7rCsF9U29WUw1 jTWknXTy1a0hvxwoaAksvcvDnBrSZsk4NatXVIfQpTeW9Oux+Wv3L1r+8w22R8j0w03L uIQykQVyjbfKDLZpjGqtn7isr7wh0TqJSVSPNfxNYkopyTxdGnX8BqXf2OzaErj+WPm4 OW84Cag69UgjNLugrtaI2eUvgS1mv0rx/lpHZL9wps+AWTwymtKeuJRwqij95OtHRpE0 S3GA== X-Gm-Message-State: AO0yUKWHmdzRWGPEjn5FVN+mMVWmZAfJIEgOJmbed+o9z9um62+BlH7q GzJtULKCdiqkN0YV0tu3F1xJ8kovTLlpSg== X-Google-Smtp-Source: AK7set/O5lIn3Mv3MdWQfmwxrplSR9oKnPmo9DhTcX9QIsh+JRC0ewIuw2EjIi0+QSiu6Sh1GVsUSg== X-Received: by 2002:a7b:ce87:0:b0:3ed:2eb5:c2e8 with SMTP id q7-20020a7bce87000000b003ed2eb5c2e8mr11558726wmj.10.1680012281233; Tue, 28 Mar 2023 07:04:41 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:40 -0700 (PDT) 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 v8 5/9] config API: have *_multi() return an "int" and take a "dest" Date: Tue, 28 Mar 2023 16:04:24 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 take advantage of 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. For now we don't make use of those new return values, but faithfully convert existing API users. 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 b7251840e20..b87fb53a215 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1511,8 +1511,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; @@ -1578,11 +1577,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 4693385e8ed..4e04efa5a72 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -185,10 +185,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 ba2ec3b54ee..e8ea4533f94 100644 --- a/config.c +++ b/config.c @@ -2421,29 +2421,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) @@ -2607,11 +2612,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, @@ -2724,9 +2729,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) @@ -2873,7 +2878,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 72d83e21e3d..109c845663d 100644 --- a/config.h +++ b/config.h @@ -450,10 +450,18 @@ int git_configset_add_file(struct config_set *cs, const char *filename); /** * 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. @@ -502,8 +510,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, @@ -557,10 +566,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 ca7c81b5c9f..4c1e6fed631 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2318,7 +2318,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 2a057c35b74..85b1ccbf784 100644 --- a/submodule.c +++ b/submodule.c @@ -275,8 +275,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 cbb33ae1fff..6dc4c37444f 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -97,8 +97,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) @@ -181,8 +180,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 Tue Mar 28 14:04:25 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: 13191130 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 ABE8EC6FD18 for ; Tue, 28 Mar 2023 14:06:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232365AbjC1OGk (ORCPT ); Tue, 28 Mar 2023 10:06:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48034 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232144AbjC1OGX (ORCPT ); Tue, 28 Mar 2023 10:06:23 -0400 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 22F5DC165 for ; Tue, 28 Mar 2023 07:05:01 -0700 (PDT) Received: by mail-wm1-x336.google.com with SMTP id l15-20020a05600c4f0f00b003ef6d684102so4194657wmq.3 for ; Tue, 28 Mar 2023 07:05:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012282; 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=dDdnQ6yERIbX0aCwM5dihvL4YSD4XPKsQZRZTjX1yG4=; b=L+URwPBcZ9dDdPtYN0YEQEvRnlEmriiy0p58T55xB0Zl1PN1qpJlEqr6+ftTt6vZkt TItZNmdegal9s2yeIRjztbiXFzeHvQerENpCv3E0LOlx4Y4fg9okGmZW0tIGTmHa6hdy VYTEK88XoO6DsQROdabD9pd8moHoKSblxX669upLXzdNqJExhvY5pKQd7yoqneikbSyH bN9kNCqe7PpkS3T8IajblqToVzT3OIIMqybHNAfVy/nsTKloagOUSA46Bj/OjyXJHK0X k8tmRNAiquo3LZmzemV7vPpdl64x0rhK5Abc+JdXu+5QNIlR8Z20odvav61D8OqilewP X5rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012282; 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=dDdnQ6yERIbX0aCwM5dihvL4YSD4XPKsQZRZTjX1yG4=; b=WsYoRAIrW4Tt1yj+Z6XQhrzOoh9jat9DsFREGxr0iPkgUEGJzqZWCizOcXfRdswXiC I2YPO12DNqEsACdH3Z+/4IjD2o6lGSz72/vysMJ/kAdfx6rLaIE4XKEhZy6KAG6+DxzI 2oOcHomItRXIcs77MRD0eT1O+DR53aKUbZorRiwRWG3L7VYU6n3RuHeljghwSERgfMpr EpASD+PEe5ZlBU4Z+sbUAAvcwQ51xqY0YpdLz8Nmgv/6Z88eE5cfSldxwOFR0zqoqSeL M05nJ4/T0n8jADsjcT3rtJB8u4DstqKIoEd9rN7tyolK1a14LuMhr1gApH+qa538YG8q yVCw== X-Gm-Message-State: AO0yUKV3Hx+fDcwLKlgKV1MZ2VMub031KOZlb4PIC4SpeHJvLr9Ao6s7 WSNtdPGzN+oZ/87msTOaHNdvZykeTbd4ow== X-Google-Smtp-Source: AK7set8S7buWsbTh938umWIzVMCjea+isfohTa+RW6vOkGMt2RPQfzcsHT5mf81NoMwd1Q94zJd2Cw== X-Received: by 2002:a05:600c:21da:b0:3ed:346d:452f with SMTP id x26-20020a05600c21da00b003ed346d452fmr12919422wmj.26.1680012282182; Tue, 28 Mar 2023 07:04:42 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:41 -0700 (PDT) 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 v8 6/9] for-each-repo: error on bad --config Date: Tue, 28 Mar 2023 16:04:25 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 Tue Mar 28 14:04:26 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: 13191132 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 997E9C6FD18 for ; Tue, 28 Mar 2023 14:06:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232721AbjC1OGu (ORCPT ); Tue, 28 Mar 2023 10:06:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48062 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232329AbjC1OGi (ORCPT ); Tue, 28 Mar 2023 10:06:38 -0400 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 342B5CC07 for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) Received: by mail-wm1-x334.google.com with SMTP id j18-20020a05600c1c1200b003ee5157346cso9508064wms.1 for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012283; 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=haGEaFXC8N6H13uRobelgkXdB/bRG8I9aJEAxY5PWo8=; b=Y/Oh0dEWmWTAIVs2dKF89BowMxGk1rCSfWz8eFdVRORQVxwM8Jlqka9sLm+YpRcq35 fK22QW9iHkInDlRNtd+EtCdyyrEef8RU/9sk76eSf2ouPci9yQ5luWkuP3PhdregDBCj yF+nWkW4vSw6+7SPd9NNywgRNEtfx5ngjzHq2tFkc7I8zT9xP6rxdHL+GdYR1J3nOpjB or7gitE8osotZwRM0phQUo2MBbvLCnhaSzuGQF3GK9dbgliz6p+q0b5mMXXoWTszRpku UYilUrtQ9h8IIBogrTjCZSYFe7pY5+Z/BcGWkxcHMGfWcmi0udzxIE42L+unUBuw9tyz vPuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012283; 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=haGEaFXC8N6H13uRobelgkXdB/bRG8I9aJEAxY5PWo8=; b=JaqDddLB3FjVicYxa8MiyYupzFffKkoa3HxrxOxmLCpcX6jEtcJGJtMOqomPrQb84B 1p9QCWIIJ2+iEV7/g5Nz1nghLUxSDd8aG+mMKzJSqvRAvigCBsEprr6RGoQ9/x8qPLCv nh6SoeyusR0akzXV3RYzmgRQ3ARm0Q3r/91PhsrjhrQugVT28vUkju/nrVFhOKZVGCFF i1wQ9JIfq6/1nnnXbsGriW0V/95X70uXyhy7qPLqYX32JwEf8CSLhE+OOExXQW8eWMmM yqhGpKc+S0y+hPt2XknidURC1VNIvFKnRx8TYaCqiEX02e4WkmAgp04opDoKi+KjzOSn Q+mg== X-Gm-Message-State: AAQBX9fWh/+Tm2OWaNPAKC6VrmBhA56rxgI6Zd5GXaDnefediH1Cva9w YbgQU5MjmsGQZWKgTuMSouYGlBNkVCrtIA== X-Google-Smtp-Source: AKy350ZgFRoEQxZPnsJwBcyS9rAxglE0V6xLz8CkTgB2Nzj3ydT4CmcujHMO4YP1YE8PQXBgOWIxQg== X-Received: by 2002:a7b:c009:0:b0:3ef:62c6:9930 with SMTP id c9-20020a7bc009000000b003ef62c69930mr8963915wmb.3.1680012283125; Tue, 28 Mar 2023 07:04:43 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:42 -0700 (PDT) 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 v8 7/9] config API users: test for *_get_value_multi() segfaults Date: Tue, 28 Mar 2023 16:04:26 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 Tue Mar 28 14:04:27 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: 13191131 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 39E6DC76196 for ; Tue, 28 Mar 2023 14:06:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232573AbjC1OGt (ORCPT ); Tue, 28 Mar 2023 10:06:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48194 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232721AbjC1OGi (ORCPT ); Tue, 28 Mar 2023 10:06:38 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61FA9CA20 for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id l15-20020a05600c4f0f00b003ef6d684102so4194744wmq.3 for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012284; 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=CgVFenbfe9out5/7TmY0T3116q5YHU6A0v0mNo0HA1Y=; b=F2w3sv+HamCm2zj9TDxJMv7wat0IxjRC5ccWyZ1/GgDe6qYA+xaIrat2zzrMvAXzXl jJR4iRU1rz1FaWp+ezEq9SQPqq6tqu9GuZ8GpZ5pkEb9akdDZtS9h2zQzlKoFHz3mV+N oHIeDUbJSpk5mmVtPlnBxRlmqxU/Lw14a84TIlqKnRrwWmCxVPK2d6jHKentHkNZN37I 9qPyu7fQqSiCfsZ0aHMYYSUhmk1l7tZyphI0/ApaxKokZkFfL4ATSGXuL44j8l2bFe8w ZlLF8il/jYKCfMUIgD3dA1d20ye3Yh1ukPBG5HIC8gh8+kCWh1eiknY4OGCGFWJJ5eTk S+SQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012284; 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=CgVFenbfe9out5/7TmY0T3116q5YHU6A0v0mNo0HA1Y=; b=XC9cofMD8yUfvEnzGKU51e4w3RrLc34BHh/p9Ajqsapmfpt6ujv3ogl++8fd31vOt4 KItQDAvnHsg5QWQi4Aphz5K1wC40Z2dOrer2qKC0Z1DD03y+P42M2m8iiOR3z3PrvZ6x bmQOdnR7gLz2+QK2CWNO5k7xyzPDmFkKFQCGFzOfjR44RnV0rcSM58FgLwCSKyKCopaM UQTln6fMJGeo8+1BxvXPTws0Ah2kdqylOigRTLyjBtVQsb6k1kk9SvEOHReefyr0gDLv bJEm45oYfoTyoz7qFoESuWtX3o2/eHHyP4IQLwubk7M0bPMk3HYC0LFGf8CpPXW2lRRi 2GZg== X-Gm-Message-State: AO0yUKWS6NHKZG+/qcqC7qViEj5GrtlC4gQtw+sOWifHU0iK2C2T0j6w 5ItaQPRctHvrUnBcAWqW0pY+mce4OfYCzw== X-Google-Smtp-Source: AK7set/KX2Ju09KqF5wseU6k66t/poAB/IlVgU5wnLcHKoQMw3PVdKrkytE7Z5WYE51turP5tN2z9w== X-Received: by 2002:a05:600c:25a:b0:3ed:358e:c1c2 with SMTP id 26-20020a05600c025a00b003ed358ec1c2mr12132020wmj.18.1680012284206; Tue, 28 Mar 2023 07:04:44 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:43 -0700 (PDT) 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 v8 8/9] config API: add "string" version of *_value_multi(), fix segfaults Date: Tue, 28 Mar 2023 16:04:27 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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 two users ofthe low-level API: - One in "builtin/for-each-repo.c", which we'll convert in a subsequent commit. - The "t/helper/test-config.c" code added in [3]. 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. 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 b87fb53a215..efc1b9a0fda 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1511,7 +1511,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; @@ -1579,8 +1579,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 4e04efa5a72..0c6556b2d78 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -187,8 +187,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 e8ea4533f94..f80a068fcad 100644 --- a/config.c +++ b/config.c @@ -2451,6 +2451,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; @@ -2619,6 +2638,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) { @@ -2734,6 +2760,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 109c845663d..4a6e3f19e5d 100644 --- a/config.h +++ b/config.h @@ -463,6 +463,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. */ @@ -513,6 +526,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, @@ -574,6 +590,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 4c1e6fed631..f8ab6be411d 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2320,7 +2320,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 85b1ccbf784..c9579f9a3f8 100644 --- a/submodule.c +++ b/submodule.c @@ -275,7 +275,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 Tue Mar 28 14:04:28 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: 13191133 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 112ADC6FD18 for ; Tue, 28 Mar 2023 14:06:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230505AbjC1OGw (ORCPT ); Tue, 28 Mar 2023 10:06:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232762AbjC1OGj (ORCPT ); Tue, 28 Mar 2023 10:06:39 -0400 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 AA2B5CC1B for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id o32so7028148wms.1 for ; Tue, 28 Mar 2023 07:05:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680012285; 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=MF0PYtt5n9HWgHVoGWzMnk9xS8YKsx0IF17BQ+px9dU=; b=bDeDbVXQxmXvwi6T86RGdrdaLw6NvZ1v9HDjxcmNf7ufu4jMfzGXVp41/AkiyhcbPj jqny+mAj1Zizg4SssbimcgtR/+xmsB472GgaETxoldCjH302JyydTlG8Uo6JdQJg37V3 NjReGwMqPeZ9HkDwm04IFDxCWziu/5VeIAS9GJ/dWutBjZ13fKmdYi/djfPlj3JxtT7k tnAPmSA/UQALPXkmZ0Ak0Z/bntico/htnftuh/k0cFtIl/mtd6TNWnkZBCuDW26pUCmo FsMuHFFxlhRfi8D1JtRasS+QAaGFmRSmM2+PYhkm6z/rRW0d0ZqWHL69/cT/LMlXv0cA B7Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680012285; 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=MF0PYtt5n9HWgHVoGWzMnk9xS8YKsx0IF17BQ+px9dU=; b=toY5pIzGd/EfGGE5P3iTgIOELfaqwqaJhCQTDC7SrFxsT1ZXOGtY1DBJ9pI/d0Tviq lV1wNBG6vxZiuaT+gjvQZ6m6Q6YOkVae3TWe2uXVCkUWcnycTxKTeTLlejmn6nain+Uk nm+mrNavhqTGG6mvqBdCoeZZ4BikRJ6VCMrZy0PbfUxkFf7NxuXk4e5iSBWAE9Z60ltx 4jWTRW/GcRtJb3HaI63Bts53SIGgkDOUr2L+FhjqxlTrjmR9mtr4ofM/L3dCi2KYTBHd Aq9npjlPgs2G7Wbogb0z9TOmp7ipV+FAvlDlLxHLyFg/UMoMN3oTIjb7Do0834gV7d5Y yk6g== X-Gm-Message-State: AAQBX9eHXV4dwJqvAS8vKH8+J8ysX/NyESDFDMKKte2jZkRyEKzKf3YL aqPFUvpIMRrnqOXwgX1+RnYw8pVbQOnFtQ== X-Google-Smtp-Source: AKy350bVXFXrDfVujxK7QJOgtJQm4QhnfSIxOQdcc2H3EnayzAzHL26JVk9phwOvXn2GhLkcrKJ9bg== X-Received: by 2002:a7b:cd8f:0:b0:3ef:6396:d9c8 with SMTP id y15-20020a7bcd8f000000b003ef6396d9c8mr8394274wmj.5.1680012285322; Tue, 28 Mar 2023 07:04:45 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c3b8a00b003ede3f5c81fsm12903622wms.41.2023.03.28.07.04.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Mar 2023 07:04:44 -0700 (PDT) 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 v8 9/9] for-each-repo: with bad config, don't conflate and Date: Tue, 28 Mar 2023 16:04:28 +0200 Message-Id: X-Mailer: git-send-email 2.40.0.rc1.1034.g5867a1b10c5 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