[PATCH v6 0/9] config API: make "multi" safe, fix segfaults, propagate "ret"
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 7, 2023, 18:09 UTC
- Message-ID
- <cover-v6-0.9-00000000000-20230307T180516Z-avarab@gmail.com>
- In-Reply-To
- <cover-v5-00.10-00000000000-20230207T154000Z-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 v5[3]:
* Drop the 6th commit, which made existing API functions return -1. We shouldn't coerce errors to "return 1", but making the API consistent can wait for a follow-up to this topic.
This should address the reason for this topic being stalled for a while, see e.g. [4].
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-v5-00.10-00000000000-20230207T154000Z-avarab@gmail.com/ 4. https://lore.kernel.org/git/xmqqcz5snyxz.fsf@gitster.g/
Æ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 v5:
1: cefc4188984 = 1: 43fdb0cf50c config tests: cover blind spots in git_die_config() tests
2: 91a44456327 = 2: 4b0799090c9 config tests: add "NULL" tests for *_get_value_multi()
3: 4a73151abde = 3: 62fe2f04e71 config API: add and use a "git_config_get()" family of functions
4: 382a77ca69e = 4: e36303f4d3d versioncmp.c: refactor config reading next commit
5: 8f17bf8150c ! 5: e38523267e7 config API: have *_multi() return an "int" and take a "dest"
@@ config.c: void git_die_config(const char *key, const char *err, ...)
}
## config.h ##
-@@ config.h: int git_configset_add_parameters(struct config_set *cs);
+@@ config.h: 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
6: b515ff13f9b < -: ----------- config API: don't lose the git_*get*() return values
7: 8a83c30ea78 = 6: 3a87b35e114 for-each-repo: error on bad --config
8: d9abc78c2be = 7: 66b7060f66f config API users: test for *_get_value_multi() segfaults
9: 65fa91e7ce7 = 8: 0da4cdb3f6a config API: add "string" version of *_value_multi(), fix segfaults
10: 4db3c6d0ed9 = 9: 627eb15a319 for-each-repo: with bad config, don't conflate <path> and <cmd>-- 2.40.0.rc1.1034.g5867a1b10c5