git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/5] *: expect a non-NULL list of config values

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 27, 2022, 14:08 UTC
Message-ID
<e06cb4df081bc2222731f9185a22ed7ad67e3814.1664287711.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1369.git.1664287711.gitgitgadget@gmail.com>
From: Derrick Stolee <derrickstolee@github.com>

The previous change altered the multi-valued config API to return an empty list intead of a NULL list. Update all callers to rely on the non-NULL result and instead check for an empty list.

Several callers only checked for a NULL list so they could safely call for_each_string_list_item(), which no-ops on an empty list.

The bitmap_preferred_tips() method is a thin wrapper on the config API, so its callers are also updated to match this expectation.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 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 | 10 ++--------
 config.c                    |  2 +-
 pack-bitmap.c               |  3 ---
 t/helper/test-config.c      |  2 +-
 9 files changed, 21 insertions(+), 50 deletions(-)
diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index fd86e5a8619..635ea5e15fd 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -44,14 +44,6 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 
 	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)
-		return 0;
-
 	for (i = 0; !result && i < values->nr; i++)
 		result = run_command_on_repo(values->items[i].string, argc, argv);
 
diff --git a/builtin/gc.c b/builtin/gc.c
index 7a585f0b71d..1e9ac2ac7e3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1493,12 +1493,10 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
 		git_config_set("maintenance.strategy", "incremental");
 
 	list = git_config_get_value_multi(key);
-	if (list) {
-		for_each_string_list_item(item, list) {
-			if (!strcmp(maintpath, item->string)) {
-				found = 1;
-				break;
-			}
+	for_each_string_list_item(item, list) {
+		if (!strcmp(maintpath, item->string)) {
+			found = 1;
+			break;
 		}
 	}
 
@@ -1550,12 +1548,10 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
 				   options);
 
 	list = git_config_get_value_multi(key);
-	if (list) {
-		for_each_string_list_item(item, list) {
-			if (!strcmp(maintpath, item->string)) {
-				found = 1;
-				break;
-			}
+	for_each_string_list_item(item, list) {
+		if (!strcmp(maintpath, item->string)) {
+			found = 1;
+			break;
 		}
 	}
 
diff --git a/builtin/log.c b/builtin/log.c
index ee19dc5d450..719ef966045 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -182,15 +182,13 @@ 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;
+	struct string_list_item *item;
 	const struct string_list *config_exclude =
 			git_config_get_value_multi("log.excludeDecoration");
 
-	if (config_exclude) {
-		struct string_list_item *item;
-		for_each_string_list_item(item, config_exclude)
-			string_list_append(decoration_filter->exclude_ref_config_pattern,
-					   item->string);
-	}
+	for_each_string_list_item(item, config_exclude)
+		string_list_append(decoration_filter->exclude_ref_config_pattern,
+				   item->string);
 
 	/*
 	 * By default, decorate_all is disabled. Enable it if
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3658c05cafc..d15e1857e67 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3982,9 +3982,6 @@ static void mark_bitmap_preferred_tips(void)
 	const struct string_list *preferred_tips;
 
 	preferred_tips = bitmap_preferred_tips(the_repository);
-	if (!preferred_tips)
-		return;
-
 	for_each_string_list_item(item, preferred_tips) {
 		for_each_ref_in(item->string, mark_bitmap_preferred_tip, NULL);
 	}
diff --git a/builtin/repack.c b/builtin/repack.c
index a5bacc77974..1655747f6e0 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -539,6 +539,7 @@ static int midx_snapshot_ref_one(const char *refname UNUSED,
 static void midx_snapshot_refs(struct tempfile *f)
 {
 	struct midx_snapshot_ref_data data;
+	struct string_list_item *item;
 	const struct string_list *preferred = bitmap_preferred_tips(the_repository);
 
 	data.f = f;
@@ -549,14 +550,10 @@ static void midx_snapshot_refs(struct tempfile *f)
 		 die(_("could not open tempfile %s for writing"),
 		     get_tempfile_path(f));
 
-	if (preferred) {
-		struct string_list_item *item;
-
-		data.preferred = 1;
-		for_each_string_list_item(item, preferred)
-			for_each_ref_in(item->string, midx_snapshot_ref_one, &data);
-		data.preferred = 0;
-	}
+	data.preferred = 1;
+	for_each_string_list_item(item, preferred)
+		for_each_ref_in(item->string, midx_snapshot_ref_one, &data);
+	data.preferred = 0;
 
 	for_each_ref(midx_snapshot_ref_one, &data);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1ddc08b076c..5a8b6120157 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -531,7 +531,6 @@ static int module_init(int argc, const char **argv, const char *prefix)
 	struct init_cb info = INIT_CB_INIT;
 	struct pathspec pathspec = { 0 };
 	struct module_list list = MODULE_LIST_INIT;
-	const struct string_list *active_modules;
 	int quiet = 0;
 	struct option module_init_options[] = {
 		OPT__QUIET(&quiet, N_("suppress output for initializing a submodule")),
@@ -553,9 +552,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 &&
-	    (active_modules = git_config_get_value_multi("submodule.active")) &&
-	    active_modules->nr)
+	if (!argc && git_config_get_value_multi("submodule.active")->nr)
 		module_list_active(&list);
 
 	info.prefix = prefix;
@@ -2709,7 +2706,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
 		opt.warn_if_uninitialized = 1;
 
 	if (opt.init) {
-		const struct string_list *active_modules;
 		struct module_list list = MODULE_LIST_INIT;
 		struct init_cb info = INIT_CB_INIT;
 
@@ -2724,9 +2720,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
 		 * If there are no path args and submodule.active is set then,
 		 * by default, only initialize 'active' modules.
 		 */
-		if (!argc &&
-		    (active_modules = git_config_get_value_multi("submodule.active")) &&
-		    active_modules->nr)
+		if (!argc && git_config_get_value_multi("submodule.active")->nr)
 			module_list_active(&list);
 
 		info.prefix = opt.prefix;
diff --git a/config.c b/config.c
index 2d4ca1ae6dc..8bd55688352 100644
--- a/config.c
+++ b/config.c
@@ -2407,7 +2407,7 @@ int git_configset_get_value(struct config_set *cs, const char *key, const char *
 	 */
 	values = git_configset_get_value_multi(cs, key);
 
-	if (!values || !values->nr)
+	if (!values->nr)
 		return 1;
 	*value = values->items[values->nr - 1].string;
 	return 0;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 9a208abc1fd..77d02c7bbe7 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2310,9 +2310,6 @@ int bitmap_is_preferred_refname(struct repository *r, const char *refname)
 	const struct string_list *preferred_tips = bitmap_preferred_tips(r);
 	struct string_list_item *item;
 
-	if (!preferred_tips)
-		return 0;
-
 	for_each_string_list_item(item, preferred_tips) {
 		if (starts_with(refname, item->string))
 			return 1;
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 62644dd71d7..90810946783 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -96,7 +96,7 @@ int cmd__config(int argc, const char **argv)
 		}
 	} else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) {
 		strptr = git_config_get_value_multi(argv[2]);
-		if (strptr && strptr->nr) {
+		if (strptr->nr) {
 			for (i = 0; i < strptr->nr; i++) {
 				v = strptr->items[i].string;
 				if (!v)

-- 
gitgitgadget
Previous: Taylor BlauNext: Junio C Hamano
Message 18 of 21 in “[RFC] config API: return empty list, not NULL”
  1. 0/5 [RFC] config API: return empty list, not NULLDerrick Stolee via GitGitGadget, Sep 27, 2022
  2. 1/5 config: relax requirements on multi-value returnDerrick Stolee via GitGitGadget, Sep 27, 2022
  3. Junio C HamanoSep 27, 2022
  4. 3/5 config: add BUG() statement instead of possible segfaultDerrick Stolee via GitGitGadget, Sep 27, 2022
  5. Ævar Arnfjörð BjarmasonSep 27, 2022
  6. Derrick StoleeSep 27, 2022
  7. Ævar Arnfjörð BjarmasonSep 27, 2022
  8. 4/5 config: return an empty list, not NULLDerrick Stolee via GitGitGadget, Sep 27, 2022
  9. Ævar Arnfjörð BjarmasonSep 27, 2022
  10. Derrick StoleeSep 27, 2022
  11. Ævar Arnfjörð BjarmasonSep 27, 2022
  12. Derrick StoleeSep 28, 2022
  13. Ævar Arnfjörð BjarmasonSep 28, 2022
  14. Derrick StoleeSep 28, 2022
  15. Ævar Arnfjörð BjarmasonSep 28, 2022
  16. 2/5 *: relax git_configset_get_value_multi resultDerrick Stolee via GitGitGadget, Sep 27, 2022
  17. Taylor BlauSep 28, 2022
  18. 5/5 *: expect a non-NULL list of config valuesDerrick Stolee via GitGitGadget, Sep 27, 2022
  19. Junio C HamanoSep 28, 2022
  20. Derrick StoleeSep 28, 2022
  21. Ævar Arnfjörð BjarmasonSep 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.