[PATCH v7 0/9] config API: make "multi" safe, fix segfaults, propagate "ret"

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Elijah Newren" <newren@gmail.com>, "Jeff King" <peff@peff.net>,
	"Taylor Blau" <me@ttaylorr.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Glen Choo" <chooglen@google.com>,
	"Calvin Wan" <calvinwan@google.com>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	raymond@heliax.dev, zweiss@equinix.com,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v7 0/9] config API: make "multi" safe, fix segfaults, propagate "ret"
Date: Wed,  8 Mar 2023 10:06:42 +0100	[thread overview]
Message-ID: <cover-v7-0.9-00000000000-20230308T090513Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v6-0.9-00000000000-20230307T180516Z-avarab@gmail.com>

This series fixes numerous segfaults in config API users, because they
didn't expect *_get_multi() to hand them a string_list with a NULL in
it given config like "[a] key" (note, no "="'s).

A larger general overview at v1[1], but note the API changes in
v2[2]. Changes since v6[3]:

 * Glen pointed out that ejecting a commit in v6 orphaned a
   corresponding forward-reference in a commit message, fix that.

1. https://lore.kernel.org/git/cover-00.10-00000000000-20221026T151328Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-v2-0.9-00000000000-20221101T225822Z-avarab@gmail.com/
3. https://lore.kernel.org/git/cover-v6-0.9-00000000000-20230307T180516Z-avarab@gmail.com/
4. https://lore.kernel.org/git/kl6lwn3sgjam.fsf@chooglen-macbookpro.roam.corp.google.com/

Ævar Arnfjörð Bjarmason (9):
  config tests: cover blind spots in git_die_config() tests
  config tests: add "NULL" tests for *_get_value_multi()
  config API: add and use a "git_config_get()" family of functions
  versioncmp.c: refactor config reading next commit
  config API: have *_multi() return an "int" and take a "dest"
  for-each-repo: error on bad --config
  config API users: test for *_get_value_multi() segfaults
  config API: add "string" version of *_value_multi(), fix segfaults
  for-each-repo: with bad config, don't conflate <path> and <cmd>

 builtin/for-each-repo.c              |  14 ++--
 builtin/gc.c                         |  15 ++--
 builtin/log.c                        |   6 +-
 builtin/submodule--helper.c          |   7 +-
 builtin/worktree.c                   |   3 +-
 config.c                             | 109 ++++++++++++++++++++++-----
 config.h                             |  68 ++++++++++++++---
 pack-bitmap.c                        |   6 +-
 submodule.c                          |   3 +-
 t/helper/test-config.c               |  28 ++++++-
 t/t0068-for-each-repo.sh             |  19 +++++
 t/t1308-config-set.sh                | 108 +++++++++++++++++++++++++-
 t/t3309-notes-merge-auto-resolve.sh  |   7 +-
 t/t4202-log.sh                       |  15 ++++
 t/t5304-prune.sh                     |  12 ++-
 t/t5310-pack-bitmaps.sh              |  20 +++++
 t/t5552-skipping-fetch-negotiator.sh |  16 ++++
 t/t7004-tag.sh                       |  17 +++++
 t/t7413-submodule-is-active.sh       |  16 ++++
 t/t7900-maintenance.sh               |  38 ++++++++++
 versioncmp.c                         |  22 ++++--
 21 files changed, 477 insertions(+), 72 deletions(-)

Range-diff against v6:
 1:  43fdb0cf50c =  1:  9f297a35e14 config tests: cover blind spots in git_die_config() tests
 2:  4b0799090c9 =  2:  45d483066ef config tests: add "NULL" tests for *_get_value_multi()
 3:  62fe2f04e71 !  3:  a977b7b188f config API: add and use a "git_config_get()" family of functions
    @@ Commit message
         "int" instead of "void". Let's leave that for now, and focus on
         the *_get_*() functions.

    -    In a subsequent commit we'll fix the other *_get_*() functions to so
    -    that they'll ferry our underlying "ret" along, rather than normalizing
    -    it to a "return 1". But as an intermediate step to that we'll need to
    -    fix git_configset_get_value_multi() to return "int", and that change
    -    itself is smaller because of this change to migrate some callers away
    -    from the *_value_multi() API.
    -
         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()
 4:  e36303f4d3d =  4:  3a5a323cd91 versioncmp.c: refactor config reading next commit
 5:  e38523267e7 =  5:  dced12a40d2 config API: have *_multi() return an "int" and take a "dest"
 6:  3a87b35e114 =  6:  d910f7e3a27 for-each-repo: error on bad --config
 7:  66b7060f66f =  7:  57db0fcd91f config API users: test for *_get_value_multi() segfaults
 8:  0da4cdb3f6a =  8:  b374a716555 config API: add "string" version of *_value_multi(), fix segfaults
 9:  627eb15a319 =  9:  6791e1f6f85 for-each-repo: with bad config, don't conflate <path> and <cmd>
-- 
2.40.0.rc1.1034.g5867a1b10c5