Skip to content

RFC config API: return empty list, not NULL

Derrick Stolee via GitGitGadget git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 RFC Latest5 patches 15 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/5config: relax requirements on multi-value returnconfig.c+1 -21 reply
2/5*: relax git_configset_get_value_multi resultbuiltin/submodule--helper.c submodule.c t/helper/test-config.c +1 more+15 -91 reply
3/5config: add BUG() statement instead of possible segfaultconfig.c+7 -23 replies
4/5config: return an empty list, not NULLconfig.c config.h+5 -47 replies
5/5*: expect a non-NULL list of config valuesbuiltin/for-each-repo.c builtin/gc.c builtin/log.c +6 more+21 -500 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/5] [RFC] config API: return empty list, not NULL

This work changes the behavior of asking for a multi-valued config key to return an empty list instead of a NULL value. This simplifies the handling of the result and is safer for development in the future.

This is based on v4 of my unregister series [1]

[1]
https://lore.kernel.org/git/[email protected]/

This idea came about due to a bug in the git maintenance unregister work where the result from git_config_get_value_multi() was sent directly to for_each_string_list_item() without checking for a NULL value first.

I'm sending this as an RFC mostly because I'm not 100% sure this shift is worth the refactoring pain and effort. I personally think getting an empty list is a safer choice, but I can also understand if someone has a different opinion.

Thanks, -Stolee

Derrick Stolee (5):
config: relax requirements on multi-value return *: relax git_configset_get_value_multi result
config: add BUG() statement instead of possible segfault
config: return an empty list, not NULL
*: expect a non-NULL list of config values

 builtin/for-each-repo.c     |  8 --------
 builtin/gc.c                | 20 ++++++++------------
 builtin/log.c               | 10 ++++------
 builtin/pack-objects.c      |  3 ---
 builtin/repack.c            | 13 +++++--------
 builtin/submodule--helper.c |  4 ++--
 config.c                    | 15 ++++++++++-----
 config.h                    |  6 +++---
 pack-bitmap.c               |  3 ---
 submodule.c                 |  2 +-
 t/helper/test-config.c      |  4 ++--
 versioncmp.c                |  8 ++++----
 12 files changed, 39 insertions(+), 57 deletions(-)

base-commit: 73a262cdca46a45aeeda6f47ea3357aaeb937e7b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1369%2Fderrickstolee%2Fconfig-empty-list-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1369/derrickstolee/config-empty-list-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1369

Signature · 1 line
gitgitgadget