From patchwork Fri Nov 25 09:50:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055687 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 252D1C4332F for ; Fri, 25 Nov 2022 09:54:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230198AbiKYJyp (ORCPT ); Fri, 25 Nov 2022 04:54:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40898 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230187AbiKYJyZ (ORCPT ); Fri, 25 Nov 2022 04:54:25 -0500 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A635547336 for ; Fri, 25 Nov 2022 01:51:50 -0800 (PST) Received: by mail-wr1-x430.google.com with SMTP id b12so5976921wrn.2 for ; Fri, 25 Nov 2022 01:51:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=K97VYJtXJU/6Zcf2HJgJN9sBULrzaTMz4Baw3NiWSg4=; b=agGuu+NYgF+STjuT+BYpNlQu1QmzkEnp41/ikr9+oTUdP5xa4/jotPUS6YqzZWCCsz XGJa/qRqkup5DQZHnr8r8vknUbNFc+gerZV2Ufki9bhNvWIFTG4ieaa6xgovzHQwkifk CrrMe9bs4lNRe27rqQKRReQR/r/gKC8PXvPoJbqxjDPqlnzwWlzJfLyLEPomu9NZty4K pRBLwoHSz02/nPy7s8h3Ykxw9gsDkfJXnHq1+snofB/+ugrmME7sC3lmiEjmf3ExBWIt Ji6Jz2tTp8E8fZHxgOBMjvT9KOPVlrvUS0XEogRFgjWR3A+PhDMR9xEnc5YFCN/VaLO5 SxZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=K97VYJtXJU/6Zcf2HJgJN9sBULrzaTMz4Baw3NiWSg4=; b=eqicMxiryPBuMYiTiHwzeGNiMmXj1cB4NBZGD5knWpSonqNk3S14R7Tvvx92Flu5jK 83+PEU0oqQ0mZBGAsP3ppxyCgh+VgKS0ZpI7TAy/4HT3pOccweiLb7eIDwxugjDwYigI pypzW2DlNK0XZkVD04h93fiJ/MNLgDzH21eCG5eG1KpzkiFcIlFgVRmAHe0BL63+6c0+ /WWQdM2JS/SNGRyDH5+BymVIKXW18UhvlXLprlZb136dUQMtmWXgspafKyBO+/+x71ki vdVpQYXnM0KLjLRe2YiL3PdFdPBbhL+SBQTrfeGNNaNPpd8Aq6L6ht5JTQS6OQvAPUC0 TjJg== X-Gm-Message-State: ANoB5pnj6WOjyQ8gbFG/04lnT8asQ6RIUPZOhr7VaFwF39ch3MKaksPe 2/LErleeCW97QQ6pV+bmMUGe8QbrDFPvsgUR X-Google-Smtp-Source: AA0mqf6OfTd5A4rQ0HvEKno/na++f5rJkqZtH7/K1NzsaqtIIK04na+HpLOsJQ8i2wMSUy14FrT11g== X-Received: by 2002:a05:6000:124d:b0:242:10a:6667 with SMTP id j13-20020a056000124d00b00242010a6667mr3664821wrx.39.1669369882507; Fri, 25 Nov 2022 01:51:22 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:21 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 1/9] for-each-repo tests: test bad --config keys Date: Fri, 25 Nov 2022 10:50:02 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 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 it's been conflating that with bad config keys. A subsequent commit will address that, but for now let's fix the gaps in test coverage, and show what we're currently doing in these cases. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t0068-for-each-repo.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index c6e0d655630..a099abc652e 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -39,4 +39,10 @@ test_expect_success 'do nothing on empty config' ' git for-each-repo --config=bogus.config -- help --no-such-option ' +test_expect_success 'bad config keys' ' + git for-each-repo --config=a && + git for-each-repo --config=a.b. && + git for-each-repo --config="'\''.b" +' + test_done From patchwork Fri Nov 25 09:50:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055690 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 2B4A9C4332F for ; Fri, 25 Nov 2022 09:54:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229752AbiKYJy4 (ORCPT ); Fri, 25 Nov 2022 04:54:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40976 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230103AbiKYJyc (ORCPT ); Fri, 25 Nov 2022 04:54:32 -0500 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D2BC48417 for ; Fri, 25 Nov 2022 01:51:53 -0800 (PST) Received: by mail-wr1-x42e.google.com with SMTP id x17so5952816wrn.6 for ; Fri, 25 Nov 2022 01:51:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=jBI2XrDpwLHXbAoJ0T0b3JqX5iW/bHQ+RbCJX8+591g=; b=CX6+iiCezlWkyx8j81/VK55ogg76yligc/P96JDoCcJtPfHBiz+QCvBq+PmbxMEL2V kzsbDld+WIjLY6WKH9h578Are7uJYgDMjc+YQdvVrBMkwt55kLCV3ZlFaVd8JiXn+ayL WVJra7mLj81FVTt1uQnZ2qt/moOYtiNIyuckVN+iAbyu09cOlaNC8Bh4/dIXwpD9FoaN b0UEK+b6va1SK4JAqJjKEOoNEk0lKH2fQMOkIgu4Qv7tosoPmUMaJN6JHO9/lcRNby2p 668suHbWozTA8pk6onBM87yxnfWEnvZB8Dkn6RzUhqzin+s3Zl49SxDIQHTRMEn2RGVv j+zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jBI2XrDpwLHXbAoJ0T0b3JqX5iW/bHQ+RbCJX8+591g=; b=qCmTzwGBLBEVFjpNcSlYXyZG4LX3V0HGj9AUnGS+boursXQ0vlnisGhBq+8nRZcFwe Vs3Dhnv7nxmrcUYIIG9g32+nLiA6e4yzG04H6WsHUSMx6I/XYijlPF2E3eZFUbrP13YK n0ctkSaGlJKP0HBczCVEtBWITD3p3Rc02xe57PYGnL8H05O6Ro7fXqLeLB8czK/Lrcj4 v8bOwe5PBSPEmM1JgXV1MRas+Hq4MgDjINKZN23WbIKxYm5tBNjQdDNgOe7RzpuON5zd 6/8drN5hkJtjSM++/azI8Ki8OYiwlsA0wnsDZA4zJ5q5CQ4Pv7NL3EgUeoU7vhURcCZn Ev8A== X-Gm-Message-State: ANoB5pmw2Qz49/A+Zi+9Wa0tqR9/r1h30ub8/jqkE2qLzaHVZ2E7goq5 Vs2we7Bi2HtvNKNIGTnfPTZamTITa5SjPfs/ X-Google-Smtp-Source: AA0mqf55pgbXpiiDyLyniu4m6C1IDwGsHPNEbap3TCf1PQpf/qrOVpYfsxBGbRATZkuzQdtpJ4g1tQ== X-Received: by 2002:a05:6000:12d1:b0:236:71fe:c9c5 with SMTP id l17-20020a05600012d100b0023671fec9c5mr22010431wrx.677.1669369883400; Fri, 25 Nov 2022 01:51:23 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:22 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 2/9] config tests: cover blind spots in git_die_config() tests Date: Fri, 25 Nov 2022 10:50:03 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 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. Let's check for that by extending the existing tests, and adding a new one for "fetch.negotiationAlgorithm" so that we have a test for a user of git_config_get_string*() calling git_die_config(). The other ones are testing: - For *-resolve.sh: A custom call to git_die_config(), or via git_config_get_notes_strategy() - For *-prune.sh: A call via git_config_get_expiry(). 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 8ae314af585..c8fa962b397 100755 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -64,8 +64,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 Fri Nov 25 09:50:04 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055689 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 5B5CEC4167B for ; Fri, 25 Nov 2022 09:54:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229740AbiKYJyy (ORCPT ); Fri, 25 Nov 2022 04:54:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229763AbiKYJyb (ORCPT ); Fri, 25 Nov 2022 04:54:31 -0500 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF6BB43AFF for ; Fri, 25 Nov 2022 01:51:54 -0800 (PST) Received: by mail-wr1-x42f.google.com with SMTP id q7so5067822wrr.8 for ; Fri, 25 Nov 2022 01:51:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=8hW2KTyxus2ppvmUB3uvyKVcUx/GbrOjdVbG8l84bF8=; b=qkwllmOG2R7dk1d9RLW6F08SKG9E1pQluX7vm+1CLZzGt9gF3t9Vw7QipuyhDO3iRe qbZxaQ1tmHatJk4zJe5wjk8nUV8kvR8H0wr36hcq54YEBfGvbqAjl0LrxmUcKFFvGlWY Ov2KgNCG8HEvthXyP+RmMJhjfmqIZQOvSqVFvPh74d+lSmtcVR6My73s3JG7STc5H/94 UT72zy0A5qk8prHwXahu9nzWnUMYwCux56erdHJ1XZaTs/wpQgW2xY21z0Okxk8xsux4 XDeQBCgUj3c+8tJsWS475ilK2EW4Yp6AJR02qhYBtjJ9oFpvxR5T979TtWQF4PS78CBw MyPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8hW2KTyxus2ppvmUB3uvyKVcUx/GbrOjdVbG8l84bF8=; b=ZePocbAp7/2fYAo7Mb6IFqiGrpOCKw/c02+KRabO9NxXYgDCe7dv2acUCNBrzAlu8e 7//BAe6nxWgebwaJGtLdaWcDzclgjocjFhaoe5nhTu7oiQ5t8pmV2ImqbOrFYqf5FYQt MXMH9VLBQz4+oOaUR8GcuC0jDuXgjsGOncdU650OMC4XCI+wULyp1uMWSn3RMLqXQdDw 70WKxk4gj+r4F9IrGxl6+OZgpf/cl9KCjOBmVcAR4BZO0wraw1WoJ073AJQJSTgu1J6v G7ZANAl/4Pl1ZPBU//DziYI1Y33ZPJuOJ3at/3hb1Q3MuMG5E+kEUQCu+tPJgLgamKN4 j6ow== X-Gm-Message-State: ANoB5pkn5/mLkh7uwSJQZPRcBiDo3R44IrBMQPgr9phaxLk7m24xu07F L+f5mt5UyGqUWgFmYyFQ9t8sUVo2vtbIHTi2 X-Google-Smtp-Source: AA0mqf5Lkr0UUtjPvTkLyZCEmACQoAqR8aR5rNGSTMF5UDGvuIk9CuKoFZ1Q35ZjCRoZRIfYapmO5A== X-Received: by 2002:a5d:5702:0:b0:242:569:3028 with SMTP id a2-20020a5d5702000000b0024205693028mr1481267wrv.435.1669369884519; Fri, 25 Nov 2022 01:51:24 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:23 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 3/9] config tests: add "NULL" tests for *_get_value_multi() Date: Fri, 25 Nov 2022 10:50:04 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A less well known edge case in the config format is that keys can be value-less, a shorthand syntax for "true" boolean keys. I.e. these two are equivalent as far as "--type=bool" is concerned: [a]key [a]key = true But as far as our parser is concerned the values for these two are NULL, and "true". I.e. for a sequence like: [a]key=x [a]key [a]key=y We get a "struct string_list" with "string" members with ".string" values of: { "x", NULL, "y" } This behavior goes back to the initial implementation of git_config_bool() in 17712991a59 (Add ".git/config" file parser, 2005-10-10). When the "t/t1308-config-set.sh" tests were added in [1] only one of the three "(NULL)" lines in "t/helper/test-config.c" had any test coverage. This change adds tests that stress the remaining two. 1. 4c715ebb96a (test-config: add tests for the config_set API, 2014-07-28) Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t1308-config-set.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index b38e158d3b2..561e82f1808 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -146,6 +146,36 @@ test_expect_success 'find multiple values' ' check_config get_value_multi case.baz sam bat hask ' +test_expect_success 'emit multi values from configset with NULL entry' ' + test_when_finished "rm -f my.config" && + cat >my.config <<-\EOF && + [a]key=x + [a]key + [a]key=y + EOF + cat >expect <<-\EOF && + x + (NULL) + y + EOF + test-tool config configset_get_value_multi a.key my.config >actual && + test_cmp expect actual +' + +test_expect_success 'multi values from configset with a last NULL entry' ' + test_when_finished "rm -f my.config" && + cat >my.config <<-\EOF && + [a]key=x + [a]key=y + [a]key + EOF + cat >expect <<-\EOF && + (NULL) + EOF + test-tool config configset_get_value a.key my.config >actual && + test_cmp expect actual +' + test_expect_success 'find value from a configset' ' cat >config2 <<-\EOF && [case] From patchwork Fri Nov 25 09:50:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055688 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 7A582C4332F for ; Fri, 25 Nov 2022 09:54:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230050AbiKYJyx (ORCPT ); Fri, 25 Nov 2022 04:54:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229776AbiKYJyb (ORCPT ); Fri, 25 Nov 2022 04:54:31 -0500 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B226945091 for ; Fri, 25 Nov 2022 01:51:54 -0800 (PST) Received: by mail-wr1-x434.google.com with SMTP id n3so5961203wrp.5 for ; Fri, 25 Nov 2022 01:51:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=lnU6EllX4TdTaD4rwWXJlLjdFETlIhF17oF2zscSWwE=; b=NTXLdhuJRaTnHPQ0nfB7rkMIkvF+kZcqNwRj0paUUXiJd1vrE+OMcTc85q+kG6Wxo/ E7pbxsEkm/4lBPC6oVRqAYc7vlnVreg+Qr8XlFp26E+GXyfShnlrBQzEq7l9pLAlWRwo qBPH+ES9A2F70a3fM98ejY3LVdkfqm5aJt8wjkeVET2zufGhTIx3yW8Kb/xVIg5aMz1j 3BwfTnZcxHxzmFGSP90W0n0v7PhPwo9RWPZa+Jqt2DMH1PRkrvUrY1Wq7NaDw0gPzuFC 3Yeqp6YPOEW8KxYuskzww8AVaOh8Nz0sTfm0XLXpm/0jo8YiYcBNl1lD422oQmAfXCYw alwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lnU6EllX4TdTaD4rwWXJlLjdFETlIhF17oF2zscSWwE=; b=Ln08IvRClLRfPz6EcTZckT9WHS9HCpzuSapFAABJHW5mupiP3OGEdxuoEA2ldRe7bP MDFwFM4U2vVYHq1s+1Sio5KCGadfI6nAEDycVaExlC2Kc/GyE12rovzKlt8yYCQ2WrVV kMIw7znm8TsbYG/Ts9flZUY09uVrQNRcFFIGqcjZESP0oOACG3+pWiaE4yqrfToHt5on VVLFEdudnYKOV56SXhq37VraVXGkmbn/XpA7lB2rOL+pddPJmG47B7wZng68bp/4n2ta 7zLfp1lA7XaHox1hWJmNFVrnQMQBFXZFqL0MT6BSit9ckwfpwcJHRoN5zb68n2SqeLmG qbGQ== X-Gm-Message-State: ANoB5pkndZ/3DQAgrrOY/Kd5gja7G0p+QUym4mUemT6rl864JWNu8m1h kPPZtlS86sdLlqdIwlX/F+zqRTn7e/c6G8Qk X-Google-Smtp-Source: AA0mqf4fNhj140eZB0Pln6tW1nITKkavGD6ZCpRDBypShfXxV4NjuzPjfbF0zevAuKO8PY7sAjfEig== X-Received: by 2002:a5d:5f04:0:b0:241:e9a6:fb3 with SMTP id cl4-20020a5d5f04000000b00241e9a60fb3mr7921895wrb.462.1669369885415; Fri, 25 Nov 2022 01:51:25 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:24 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 4/9] versioncmp.c: refactor config reading next commit Date: Fri, 25 Nov 2022 10:50:05 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 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 ta avoid repeating ourselves. Let's also split out the names of the config variables into variables of our own, so we don't have to repeat ourselves, and refactor the nested if/else to avoid indenting it, and the existing bracing style issue. This all helps with the subsequent commit, where we'll need to start checking different git_config_get_value_multi() return value. See c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) for the original implementation of most of this. Moving the "initialized = 1" assignment allows us to move some of this to the variable declarations in the subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason --- versioncmp.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/versioncmp.c b/versioncmp.c index 069ee94a4d7..323f5d35ea8 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -160,15 +160,18 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { - const struct string_list *deprecated_prereleases; + const char *const newk = "versionsort.suffix"; + const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *oldl; + + prereleases = git_config_get_value_multi(newk); + oldl = git_config_get_value_multi(oldk); + if (prereleases && oldl) + warning("ignoring %s because %s is set", oldk, newk); + else if (!prereleases) + prereleases = oldl; + initialized = 1; - prereleases = git_config_get_value_multi("versionsort.suffix"); - deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); - if (prereleases) { - if (deprecated_prereleases) - warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); - } else - prereleases = deprecated_prereleases; } if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1, &diff)) From patchwork Fri Nov 25 09:50:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055692 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 26169C4332F for ; Fri, 25 Nov 2022 09:55:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229852AbiKYJy7 (ORCPT ); Fri, 25 Nov 2022 04:54:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41184 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230186AbiKYJyc (ORCPT ); Fri, 25 Nov 2022 04:54:32 -0500 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2C8E245EE8 for ; Fri, 25 Nov 2022 01:51:55 -0800 (PST) Received: by mail-wr1-x432.google.com with SMTP id s5so5976949wru.1 for ; Fri, 25 Nov 2022 01:51:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=virdkYmWkaQxITvhC8RRnAeoSCEsQ00hmb2CglYu56M=; b=eV29twYhhSoeAj7YT02s2fWzO1qq+SGv8JXFhJ4aOAqwVX1a+zYymY22qPZ5O9lTWd /e63HwQ4kV6lrRFaEbPo1QjKGm2DLxnZqtkuJvZQGjEe7FMboxt4/PUVgdufiJmGPn2s ZOpdnAqFl8lTaYPqhIfH6ACl6ypU9bvVNbcPANiKemC/k9fD7gJlJ5GjwZ2mheHnVZDD 1w4by8IDTAhAMgo4pD1/8gJDAfunbNw251oYZBLQw1BZLnBRtjdlk5VVAAfonC0mDGm3 6Mv5cVnI/KGX+37tmGrkjw/wFUDQFDATNu9bUOJE4LDnzW5Etrk/CPyB9dTm8Z2njP3P yXjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=virdkYmWkaQxITvhC8RRnAeoSCEsQ00hmb2CglYu56M=; b=odxg1B8UEz/cXYablmVDoJ7Ts0ZoSxAlhnyr4wze2146EeOgjBAYOI8fNqwifve7l+ YFg8jBUzGWoSYx8hZ1L5CwV/OpI0vNJ0eLbN6u2paj5mLJOuiLDBThsVYaGVpTwqloXF 6Eg4CvmdfKjY3YtPCYCpSXbVIcJnIF0T0xQZ2Blc5ypNG+KGaKCx+kLwnX2ITdew0sdG VLjhjy4G5QZqmZCSnntVbrld9x+B+a5qRxKNYNQFsfbtHlGsbu0EynPk8DGiCb1OQFap UJY4BO/0+6vvuHyG3yfI3f//LTuHShSR+fIyd+Tx6V3aQKg+2pBbLh26wk7IoYpLKxVG i0UA== X-Gm-Message-State: ANoB5pl3MUmZGOTggktXmHH2dAscUyJKQ+pevYl7BCrz4pHRMKEfAV51 xlSgp6ixrLVe9TBXPkji6yqJxeTL1MtSIxY6 X-Google-Smtp-Source: AA0mqf6m2hI4OysQWSKVh5CKAww/P4sPZTnIUNvu5eFrw6e5u5KT5Bs80w203u9KZTzn4DE5dV7zNQ== X-Received: by 2002:a05:6000:1370:b0:22c:dd26:3c0 with SMTP id q16-20020a056000137000b0022cdd2603c0mr23414940wrz.18.1669369886345; Fri, 25 Nov 2022 01:51:26 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:25 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 5/9] config API: have *_multi() return an "int" and take a "dest" Date: Fri, 25 Nov 2022 10:50:06 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Have the "git_configset_get_value_multi()" function and its siblings return an "int" and populate a "**dest" parameter like every other git_configset_get_*()" in the API. As we'll see in in subsequent commits this fixes a blind spot in the API where it wasn't possible to tell whether a list was empty from whether a config key existed. We'll take advantage of that in subsequent commits, but for now we're faithfully converting existing API callers. See [1] for the initial addition of "git_configset_get_value_multi()" 1. 3c8687a73ee (add `config_set` API for caching config-like files, 2014-07-28). A logical follow-up to this would be to change the various "*_get_*()" functions to ferry the git_configset_get_value() return value to their own callers, e.g. git_configset_get_int() returns "1" rather than ferrying up the "-1" that "git_configset_get_value()" might return, but that's not being done in this series Most of this is straightforward, commentary on cases that stand out: - As we've tested for in a preceding commit we can rely on getting the config list in git_die_config(), and as we need to handle the new return value let's BUG() out if we can't acquire it. - In "builtin/for-each-ref.c" 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. - 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. - We have code e.g. in "builtin/submodule--helper.c" that only wants to check if a config key exists, and would be better served with another API, but let's keep using "git_configset_get_value_multi()" for now. - In "versioncmp.c" we now use the return value of the functions, instead of checking if the lists are still non-NULL. This is strictly speaking unnecessary, but makes the API use consistent with the rest, but more importantly... - ...because we always check our return values we can assert that with the RESULT_MUST_BE_USED macro added in 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init() return values, 2022-09-01) Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 13 ++++----- builtin/gc.c | 10 +++---- builtin/log.c | 6 ++-- builtin/submodule--helper.c | 7 +++-- config.c | 55 ++++++++++++++++++++++++++----------- config.h | 29 +++++++++++++------ pack-bitmap.c | 6 +++- submodule.c | 3 +- t/helper/test-config.c | 6 ++-- versioncmp.c | 11 +++++--- 10 files changed, 92 insertions(+), 54 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 6aeac371488..7cc41847635 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,14 +46,10 @@ 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) + err = repo_config_get_value_multi(the_repository, config_key, &values); + if (err < 0) + return 0; + else if (err) return 0; for (i = 0; !result && i < values->nr; i++) diff --git a/builtin/gc.c b/builtin/gc.c index 02455fdcd73..69503e0a023 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1513,8 +1513,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) else git_config_set("maintenance.strategy", "incremental"); - list = git_config_get_value_multi(key); - if (list) { + if (!git_config_get_value_multi(key, &list)) { for_each_string_list_item(item, list) { if (!strcmp(maintpath, item->string)) { found = 1; @@ -1580,11 +1579,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 5eafcf26b49..cc9d92f95da 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -182,10 +182,10 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f int i; char *value = NULL; struct string_list *include = decoration_filter->include_ref_pattern; - const struct string_list *config_exclude = - git_config_get_value_multi("log.excludeDecoration"); + const struct string_list *config_exclude; - if (config_exclude) { + if (!git_config_get_value_multi("log.excludeDecoration", + &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) string_list_append(decoration_filter->exclude_ref_config_pattern, diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index c75e9e86b06..08c12b25375 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -541,6 +541,7 @@ static int module_init(int argc, const char **argv, const char *prefix) NULL }; int ret = 1; + const struct string_list *values; argc = parse_options(argc, argv, prefix, module_init_options, git_submodule_helper_usage, 0); @@ -552,7 +553,7 @@ static int module_init(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && git_config_get_value_multi("submodule.active")) + if (!argc && !git_config_get_value_multi("submodule.active", &values)) module_list_active(&list); info.prefix = prefix; @@ -2714,6 +2715,7 @@ static int module_update(int argc, const char **argv, const char *prefix) if (opt.init) { struct module_list list = MODULE_LIST_INIT; struct init_cb info = INIT_CB_INIT; + const struct string_list *values; if (module_list_compute(argv, opt.prefix, &pathspec2, &list) < 0) { @@ -2726,7 +2728,8 @@ static int module_update(int argc, const char **argv, const char *prefix) * If there are no path args and submodule.active is set then, * by default, only initialize 'active' modules. */ - if (!argc && git_config_get_value_multi("submodule.active")) + if (!argc && !git_config_get_value_multi("submodule.active", + &values)) module_list_active(&list); info.prefix = opt.prefix; diff --git a/config.c b/config.c index c058b2c70c3..0b07045ed8c 100644 --- a/config.c +++ b/config.c @@ -2275,23 +2275,28 @@ void read_very_early_config(config_fn_t cb, void *data) config_with_options(cb, data, NULL, &opts); } -static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) +static int configset_find_element(struct config_set *cs, const char *key, + struct config_set_element **dest) { struct config_set_element k; struct config_set_element *found_entry; char *normalized_key; + int ret; + /* * `key` may come from the user, so normalize it before using it * for querying entries from the hashmap. */ - if (git_config_parse_key(key, &normalized_key, NULL)) - return NULL; + ret = git_config_parse_key(key, &normalized_key, NULL); + if (ret < 0) + return ret; hashmap_entry_init(&k.ent, strhash(normalized_key)); k.key = normalized_key; found_entry = hashmap_get_entry(&cs->config_hash, &k, ent, NULL); free(normalized_key); - return found_entry; + *dest = found_entry; + return 0; } static int configset_add_value(struct config_set *cs, const char *key, const char *value) @@ -2300,8 +2305,11 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha struct string_list_item *si; struct configset_list_item *l_item; struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); + int ret; - e = configset_find_element(cs, key); + ret = configset_find_element(cs, key, &e); + if (ret < 0) + return ret; /* * Since the keys are being fed by git_config*() callback mechanism, they * are already normalized. So simply add them without any further munging. @@ -2395,24 +2403,38 @@ 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); + ret = git_configset_get_value_multi(cs, key, &values); - if (!values) + if (ret < 0) + return ret; + else if (!values) return 1; assert(values->nr > 0); *value = values->items[values->nr - 1].string; return 0; } -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key) +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest) { - struct config_set_element *e = configset_find_element(cs, key); - return e ? &e->value_list : NULL; + struct config_set_element *e; + int ret; + + ret = configset_find_element(cs, key, &e); + if (ret < 0) + return ret; + else if (!e) + return 1; + *dest = &e->value_list; + + return 0; } int git_configset_get_string(struct config_set *cs, const char *key, char **dest) @@ -2558,11 +2580,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, @@ -2670,9 +2692,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) @@ -2819,7 +2841,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 ef9eade6414..7f6ce6f2fb5 100644 --- a/config.h +++ b/config.h @@ -459,10 +459,18 @@ int git_configset_add_parameters(struct config_set *cs); /** * Finds and returns the value list, sorted in order of increasing priority * for the configuration variable `key` and config set `cs`. When the - * configuration variable `key` is not found, returns NULL. The caller - * should not free or modify the returned pointer, as it is owned by the cache. + * configuration variable `key` is not found, returns 1 without touching + * `value`. + * + * The key will be parsed for validity with git_config_parse_key(), on + * error a negative value will be returned. + * + * The caller should not free or modify the returned pointer, as it is + * owned by the cache. */ -const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key); +RESULT_MUST_BE_USED +int git_configset_get_value_multi(struct config_set *cs, const char *key, + const struct string_list **dest); /** * Clears `config_set` structure, removes all saved variable-value pairs. @@ -496,8 +504,9 @@ struct repository; void repo_config(struct repository *repo, config_fn_t fn, void *data); int repo_config_get_value(struct repository *repo, const char *key, const char **value); -const struct string_list *repo_config_get_value_multi(struct repository *repo, - const char *key); +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, @@ -544,10 +553,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 440407f1be7..81f0c0e016b 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2301,7 +2301,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 8ac2fca855d..e43c4230ba3 100644 --- a/submodule.c +++ b/submodule.c @@ -274,8 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - sl = repo_config_get_value_multi(repo, "submodule.active"); - if (sl) { + if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/helper/test-config.c b/t/helper/test-config.c index 4ba9eb65606..8f70beb6c9d 100644 --- a/t/helper/test-config.c +++ b/t/helper/test-config.c @@ -95,8 +95,7 @@ int cmd__config(int argc, const char **argv) goto exit1; } } else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) { - strptr = git_config_get_value_multi(argv[2]); - if (strptr) { + if (!git_config_get_value_multi(argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) @@ -159,8 +158,7 @@ int cmd__config(int argc, const char **argv) goto exit2; } } - strptr = git_configset_get_value_multi(&cs, argv[2]); - if (strptr) { + if (!git_configset_get_value_multi(&cs, argv[2], &strptr)) { for (i = 0; i < strptr->nr; i++) { v = strptr->items[i].string; if (!v) diff --git a/versioncmp.c b/versioncmp.c index 323f5d35ea8..60c3a517122 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -162,13 +162,16 @@ int versioncmp(const char *s1, const char *s2) if (!initialized) { const char *const newk = "versionsort.suffix"; const char *const oldk = "versionsort.prereleasesuffix"; + const struct string_list *newl; const struct string_list *oldl; + int new = git_config_get_value_multi(newk, &newl); + int old = git_config_get_value_multi(oldk, &oldl); - prereleases = git_config_get_value_multi(newk); - oldl = git_config_get_value_multi(oldk); - if (prereleases && oldl) + if (!new && !old) warning("ignoring %s because %s is set", oldk, newk); - else if (!prereleases) + if (!new) + prereleases = newl; + else if (!old) prereleases = oldl; initialized = 1; From patchwork Fri Nov 25 09:50:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055691 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 BFDCAC4167B for ; Fri, 25 Nov 2022 09:54:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229798AbiKYJy6 (ORCPT ); Fri, 25 Nov 2022 04:54:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230106AbiKYJyc (ORCPT ); Fri, 25 Nov 2022 04:54:32 -0500 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 487C31F2E8 for ; Fri, 25 Nov 2022 01:51:56 -0800 (PST) Received: by mail-wm1-x32c.google.com with SMTP id m7-20020a05600c090700b003cf8a105d9eso2985819wmp.5 for ; Fri, 25 Nov 2022 01:51:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=p+vAlRKZ6YB7y9RwWasF5mCe4XNpLLczSEMKdagapu4=; b=jZaOlpQBLhgyp0d/cMXIZeYsmOd3iQc3NCPh9KT7bL1DK8cEX5i3TLVbPOSJcG3Vbj Rb1AcLorY5Wb/jepBydVIPLRDpuhaQmMCXEgsEnVj8yuZ0VNwdOZcPbAPF03RB5ExPhm 1Ef/ZZod8+C99nia8W3pT7BTtO7yZHmQBQE1IDoWo8m2e5fUqoV5Hn2hufF4D5Decgca hVNxmmy1FUWYE8/gyRbEH4GdDJnFQo7co84gBmB9LS0EeqyNztMD9UhQTUE76ZKMpuND bFPmW/B08+/J8sZhZ1ZmsbLJIzTMnSDBMz2FEB4DPPeqb4FjzHv3xABoxx/ql9/ejxQ8 Y9kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=p+vAlRKZ6YB7y9RwWasF5mCe4XNpLLczSEMKdagapu4=; b=gvMPyDHSuUFB5Cxbv9FG72eXYkZUdC37ZAiI6iEr/c51QojMC/zPmPWVA7Sfzj/6+l m8F5Xo4qm10+/U2rhqtkiYX60e6szhHxUg/KgRTICQFVg/e2/ceIEajL0spGiITqudh7 rQp8DpqpyWARF4UKqhStQ9UsSfMBpj1HvRlpjzAapD+gkjMv/L0ehhj9USllhhEz3Y7E f6th71TQZG1L5ppl5ZLtms8TQBrcVcbfsr+zeMi6ChuLqrakxo7iDNSVxwaSa5YR7ukO EquBUOWrOO76S886UEyxkR0xynNyaH4EloT/ZG7xFjKNG/iCh05Zar17z0LYnQZMxmSD KFFg== X-Gm-Message-State: ANoB5pn/tGuO5J30nn6H3K7YZIDS+ThSi8S8JG0e//pWPsAAb/082JvZ DCu3/GO0+Gf7gTEl4R3WhgoczEUwHuANkLsN X-Google-Smtp-Source: AA0mqf4D20YmhwZ/riod/RPecSTIpjyTLQK26iTPhtRihq9VPIQYj+vofiuGQL3/I05/tHt2pE7/Xg== X-Received: by 2002:a05:600c:384d:b0:3cf:7217:d5fa with SMTP id s13-20020a05600c384d00b003cf7217d5famr15981158wmr.191.1669369887451; Fri, 25 Nov 2022 01:51:27 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:26 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 6/9] for-each-repo: error on bad --config Date: Fri, 25 Nov 2022 10:50:07 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 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. Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/for-each-repo.c | 3 ++- t/t0068-for-each-repo.sh | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c index 7cc41847635..224164addb3 100644 --- a/builtin/for-each-repo.c +++ b/builtin/for-each-repo.c @@ -48,7 +48,8 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix) err = repo_config_get_value_multi(the_repository, config_key, &values); if (err < 0) - return 0; + usage_msg_optf(_("got bad config --config=%s"), + for_each_repo_usage, options, config_key); else if (err) return 0; diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh index a099abc652e..19ceaa546ea 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -39,10 +39,10 @@ test_expect_success 'do nothing on empty config' ' git for-each-repo --config=bogus.config -- help --no-such-option ' -test_expect_success 'bad config keys' ' - git for-each-repo --config=a && - git for-each-repo --config=a.b. && - git for-each-repo --config="'\''.b" +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 Fri Nov 25 09:50:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055693 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 894FBC4167B for ; Fri, 25 Nov 2022 09:55:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230026AbiKYJzB (ORCPT ); Fri, 25 Nov 2022 04:55:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41056 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229575AbiKYJye (ORCPT ); Fri, 25 Nov 2022 04:54:34 -0500 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 07CE748424 for ; Fri, 25 Nov 2022 01:51:57 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id t25-20020a1c7719000000b003cfa34ea516so5818584wmi.1 for ; Fri, 25 Nov 2022 01:51:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=LWKuBVMFrCFcdf9R4hcItVFlxyZEr+3GB+LU1CF5vwA=; b=iUVSUim4Z8Pi8/su0fxkOnJmCk2XT6eIPIM0n3a9Vc4lumnyS7M9sI04pImjfI5Q8d 8JqKf+dCLeXbdQgMXw/NR5NccD9ln3l0kU6xmuNG+oRvuLE5z8Y1F/qjDjA8NMbR+uXK qeJqQPNw9TYikIJB/9UIkhC67okC8fTFXuvoz+CtS30dX2mLvnwgzqg1caDDCp8ZBrLz W4NRTdmqRi11mWc0BcTOUve/CL+35R8JHkfn7R9NNAwBAVTmUXmSsHFyK4ROYraTWvBh tImjn/swE8rBwKALn9Myt2AMFo1GBopOP9d6aIsMhdBq9Od2IDunHozgaumqRryQeu9U Uh5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=LWKuBVMFrCFcdf9R4hcItVFlxyZEr+3GB+LU1CF5vwA=; b=DfWD9GIR5m4wShkP92edifVNgPCgKT4ZW7wMWv8ss26pQ9Um5Pa+1bCJSpFyzPdimi xvJE4ZG46DQcMyK+pt/EYnwVE0IZJ3mN8s3NXPsVJj5/TtsRfk7MoEabLbJ9u/j/wZVM gvPTQ3MYuNux0dCwn2M9ZtPie2KYbLODIeEcroLR/e7OwIK0ZPSXPWwifcWYR5L6bzdW muMOtUZTqvkVB05GrGqNsY6IeOOx/betcLmzc10SG5BFtrf9u/P8vqdXirm6BLoZ53gu OtNy23gsllUut7Nqh7JsKOLq3tzCl6zPCQAxIrnWSS+zfY2iTI1DQN74QjJBh/rFzAw3 IKJg== X-Gm-Message-State: ANoB5pn+0Or+IuCDfvXcOkzMnW1izdzeELkMkbuz7llbf8Vx3ZFJuYSZ WEw4WfbP2u1H/bKdJPFhj+aTvjuaHJDbcHHT X-Google-Smtp-Source: AA0mqf5/zv+Slg0KVDqxnhU0TGVvL5D2LB92+I73hS5WLd3w7A+duHK6s7ic1KRubvvaoU5RAFNO4g== X-Received: by 2002:a05:600c:3d18:b0:3cf:b7bf:352d with SMTP id bh24-20020a05600c3d1800b003cfb7bf352dmr18943515wmb.106.1669369888389; Fri, 25 Nov 2022 01:51:28 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:27 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 7/9] config API users: test for *_get_value_multi() segfaults Date: Fri, 25 Nov 2022 10:50:08 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As we'll discus 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. 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 | 26 ++++++++++++++++++++++++++ 5 files changed, 77 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 6d693eef82f..2e65c8139c4 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..2dbac07be83 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -524,6 +524,32 @@ 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 + cat >expect <<-\EOF && + error: missing value for '\''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 Fri Nov 25 09:50:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055694 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 12BE9C4321E for ; Fri, 25 Nov 2022 09:55:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230209AbiKYJzE (ORCPT ); Fri, 25 Nov 2022 04:55:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229847AbiKYJye (ORCPT ); Fri, 25 Nov 2022 04:54:34 -0500 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 5196C4842B for ; Fri, 25 Nov 2022 01:51:57 -0800 (PST) Received: by mail-wm1-x333.google.com with SMTP id a11-20020a05600c2d4b00b003cf6f5fd9f1so3012016wmg.2 for ; Fri, 25 Nov 2022 01:51:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=hj43VK7Jg+Uew39T4EakLpqRzJ8BYIYg6M6EY59gC2Y=; b=qL4cy4z3qog64+Iuw3jeyJXoLG9U62197Qgqn4TSmF2feFU3KkXFUIXE2Glw5r6Kmn RZiTxz5BiVbqYHCS1gqqWJYuR0MD8D7pf+7BThohc/LFUEcgMDmstpti07hVZpHScyT2 3XKFPDTVufCmKSF+1aNQzKaNNeB2GyJuzcRxjmnifXf9ozXPE+WsLxITp+6EKvWQwJZT 8pT4h7qOzGdhJUcm2gudrNe4q+X/y6azHelx5sutvPX6BOVaUsQQ6C85IjfkCxk1M+be U5r6sohHi0zPYED1vQsBKbB4ZqkRXBH7hHvZRX2ldCd1EjAYCDytLQquI8okp5+f/sTz DMcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hj43VK7Jg+Uew39T4EakLpqRzJ8BYIYg6M6EY59gC2Y=; b=F2FcvVmjNZ1iWrJZsvjMwl1jSoYJHPvYPi/P7mUyhyUkHQMMp7078hfamcJLYYsawG 6KtcFf8Xq4Hn6SMVK32Xl/NHxafphAdffw1AIEmYzU2/GdwicsVzk8Lwq2NET7VDUr2h 7KcVV476AJjF6Ey+M85Kp5zJlCd7q6jRdW7FAMiRhmcoxjJa7C9fO2b7XQhyv6NJ2RPR 3vAZm/rCSW1GqQpDpuVL3PIngn1fYSuNki2Tm0x/JrxS+ACDz6pv+8au+aXHaZUEuhXa /NyV8zqZutNV01g7R6HPXWBT9/J/VYVKom5M+cw+p7wCreNgFlJivfJ0gcgA5D1MJvTx ZsZw== X-Gm-Message-State: ANoB5pladJ8LjjkpJ92CEB1vi1LfszTF7xl6nYtXTX3Ro7q8w31spyiK CabP6QtShLdo1dqK0qQWgjq0MFxHN+i3gMbb X-Google-Smtp-Source: AA0mqf7YDEDl5Gie51re8299h2sGbXpJBEuiFSzFeN2mtdFJvpOhYAQwGtLTXxuIuxyeTczEvzLk9g== X-Received: by 2002:a7b:c3c9:0:b0:3cf:5442:bbe with SMTP id t9-20020a7bc3c9000000b003cf54420bbemr30920835wmj.2.1669369889454; Fri, 25 Nov 2022 01:51:29 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:28 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 8/9] config API: add "string" version of *_value_multi(), fix segfaults Date: Fri, 25 Nov 2022 10:50:09 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix numerous and mostly long-standing segfaults in consumers of the *_config_*value_multi() API. As discussed in the preceding commit an empty key in the config syntax yields a "NULL" string, which these users would give to strcmp() (or similar), resulting in segfaults. As this change shows, most users users of the *_config_*value_multi() API didn't really want such an an unsafe and low-level API, let's give them something with the safety of git_config_get_string() instead. This fix is similar to what the *_string() functions and others acquired in[1] and [2]. Namely introducing and using a safer "*_get_string_multi()" variant of the low-level "_*value_multi()" function. This fixes segfaults in code introduced in: - d811c8e17c6 (versionsort: support reorder prerelease suffixes, 2015-02-26) - c026557a373 (versioncmp: generalize version sort suffix reordering, 2016-12-08) - a086f921a72 (submodule: decouple url and submodule interest, 2017-03-17) - a6be5e6764a (log: add log.excludeDecoration config option, 2020-04-16) - 92156291ca8 (log: add default decoration filter, 2022-08-05) - 50a044f1e40 (gc: replace config subprocesses with API calls, 2022-09-27) There are now three remaining files using the low-level API: - Two cases in "builtin/submodule--helper.c", where it's used safely to see if any config exists. - 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 | 22 +++++++++++++++++----- versioncmp.c | 4 ++-- 12 files changed, 102 insertions(+), 22 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 69503e0a023..2d95c5b29aa 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1513,7 +1513,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) else 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; @@ -1581,8 +1581,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 cc9d92f95da..cd17f311712 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -184,8 +184,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f struct string_list *include = decoration_filter->include_ref_pattern; const struct string_list *config_exclude; - if (!git_config_get_value_multi("log.excludeDecoration", - &config_exclude)) { + if (!git_config_get_string_multi("log.excludeDecoration", + &config_exclude)) { struct string_list_item *item; for_each_string_list_item(item, config_exclude) string_list_append(decoration_filter->exclude_ref_config_pattern, diff --git a/config.c b/config.c index 0b07045ed8c..9bd43189c02 100644 --- a/config.c +++ b/config.c @@ -2437,6 +2437,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_string(struct config_set *cs, const char *key, char **dest) { const char *value; @@ -2587,6 +2606,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) { @@ -2697,6 +2723,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 7f6ce6f2fb5..3079d60a860 100644 --- a/config.h +++ b/config.h @@ -472,6 +472,19 @@ RESULT_MUST_BE_USED int git_configset_get_value_multi(struct config_set *cs, const char *key, const struct string_list **dest); +/** + * A validation wrapper for git_configset_get_value_multi() which does + * for it what git_configset_get_string() does for + * git_configset_get_value(). + * + * The configuration syntax allows for "[section] key", which will + * give us a NULL entry in the "struct string_list", as opposed to + * "[section] key =" which is the empty string. Most users of the API + * are not prepared to handle NULL in a "struct string_list". + */ +int git_configset_get_string_multi(struct config_set *cs, const char *key, + const struct string_list **dest); + /** * Clears `config_set` structure, removes all saved variable-value pairs. */ @@ -507,6 +520,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, @@ -561,6 +577,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 81f0c0e016b..dd05ab03ca0 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2303,7 +2303,7 @@ const struct string_list *bitmap_preferred_tips(struct repository *r) { const struct string_list *dest; - if (!repo_config_get_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 e43c4230ba3..0f6cf864ed9 100644 --- a/submodule.c +++ b/submodule.c @@ -274,7 +274,7 @@ int is_tree_submodule_active(struct repository *repo, free(key); /* submodule.active is set */ - if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) { + if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) { struct pathspec ps; struct strvec args = STRVEC_INIT; const struct string_list_item *item; diff --git a/t/t4202-log.sh b/t/t4202-log.sh index e4f02d8208b..ae73aef922f 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -835,7 +835,7 @@ test_expect_success 'log.decorate configuration' ' ' -test_expect_failure 'parse log.excludeDecoration with no value' ' +test_expect_success 'parse log.excludeDecoration with no value' ' cp .git/config .git/config.orig && test_when_finished mv .git/config.orig .git/config && @@ -843,7 +843,11 @@ test_expect_failure 'parse log.excludeDecoration with no value' ' [log] excludeDecoration EOF - git log --decorate=short + cat >expect <<-\EOF && + error: missing value for '\''log.excludeDecoration'\'' + EOF + git log --decorate=short 2>actual && + test_cmp expect actual ' test_expect_success 'decorate-refs with glob' ' diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 2e65c8139c4..894c750080c 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 2dbac07be83..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 && @@ -546,8 +551,15 @@ test_expect_failure 'unregister with no value for maintenance.repo' ' cat >expect <<-\EOF && error: missing value for '\''maintenance.repo'\'' EOF - git maintenance unregister && - git maintenance unregister --force + 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 Fri Nov 25 09:50:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 13055695 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 6AFA7C4332F for ; Fri, 25 Nov 2022 09:55:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230116AbiKYJzW (ORCPT ); Fri, 25 Nov 2022 04:55:22 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40394 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229918AbiKYJyg (ORCPT ); Fri, 25 Nov 2022 04:54:36 -0500 Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 563FB48755 for ; Fri, 25 Nov 2022 01:52:00 -0800 (PST) Received: by mail-wr1-x433.google.com with SMTP id i12so6024577wrb.0 for ; Fri, 25 Nov 2022 01:52:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=OzYcG+zB3MBLBP80r/GiaD6DPW5CcLKPsFpk2Bj+idM=; b=lNqNpGTfK/HrAC12OAfsPzPYhBrai1Jcy/00epKq74GreEiDgGMMX0VONGegD61Ylx +iPzE6XGCd8RINmwP9G26xk4E1vHQBzXRabHI+U1DHQjs4IP2pbe/ZJAYLZuRg+O25Gs K+8N5WRn3VXremadEpM8pkeBaBwXzlifKGZjW6LOoj6+WRQ3uoJ0SXpqt6Sy1Or6yKOO QEK1yKcKp/T5XXkvffm/KRALL7zrbAGOLU740NZT0lAv7XArUdABSem9znUxXMvqwp2+ nCVU0VfMHWQ16rcB0kvJRpkdvRJfA6iQVPWRRY/AHRUZCwGULYtZ4yM22HNYomigVuyK L/kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OzYcG+zB3MBLBP80r/GiaD6DPW5CcLKPsFpk2Bj+idM=; b=77FwcnUkIFc5sO1N34+FCLgFOdCfOKc8jgxyqpX5PTcV9itwKMoDmiV4CSAPCRlDRJ 1jGAdNRf1jlmbcsh++AmoqIjlyZy4brlhUeOA4OQlAn39Lj+D+74oxPQAdoi6rIjkm0O HeB1Qza6Zaufa1AIpX2qVUD6128y5m1bD1rzlHtfxI9KDbqOTfqRxyAkLleyN9RRbIdZ FmU2v4J5B84CNkOwtlvPxpOcabCdTtj1SaW+8dUz3LhKoo2xEkYoOBdStjt7/ceRx2Lg UJ47q3zsUkjSBQlj5hVtbuQatYOheBuSB9NWPjOAh9xJwOeCBNlXJX2RPOP+WmYdLA7k qeVg== X-Gm-Message-State: ANoB5pk4bdD49OzEVlI/t/wFVIlBefNXS0TRcmabXzBxUtZSQnn9t0Q5 DjoRK5OtXKzU3Aax9NWjF3nQ355m1z4F5viE X-Google-Smtp-Source: AA0mqf5tTFI/CKMBMsi3XEtSd7ETazJvAAxMIBlPNAIQKbSHU5TClfI+pEor8x4GPblW1w5+M2M6lA== X-Received: by 2002:a5d:55c3:0:b0:236:c12b:6e29 with SMTP id i3-20020a5d55c3000000b00236c12b6e29mr22232131wrw.98.1669369890513; Fri, 25 Nov 2022 01:51:30 -0800 (PST) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id z6-20020adfe546000000b0023655e51c33sm3420975wrm.4.2022.11.25.01.51.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:51:29 -0800 (PST) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , Jeff King , Taylor Blau , =?utf-8?q?SZEDER_G=C3=A1bor?= , =?utf-8?b?w4Z2YXIg?= =?utf-8?b?QXJuZmrDtnLDsCBCamFybWFzb24=?= Subject: [PATCH v3 9/9] for-each-repo: with bad config, don't conflate and Date: Fri, 25 Nov 2022 10:50:10 +0100 Message-Id: X-Mailer: git-send-email 2.39.0.rc0.955.ge9b241be664 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 19ceaa546ea..48187a40d64 100755 --- a/t/t0068-for-each-repo.sh +++ b/t/t0068-for-each-repo.sh @@ -45,4 +45,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