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

[PATCH 4/4] config API: don't use vreportf(), make it static in usage.c

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 6, 2021, 16:55 UTC
Message-ID
<patch-4.4-e0e6427cbd3-20211206T165221Z-avarab@gmail.com>
In-Reply-To
<cover-0.4-00000000000-20211206T165221Z-avarab@gmail.com>

In preceding commits the rest of the vreportf() users outside of usage.c have been migrated to die_message(), leaving only the git_die_config() function added in 5a80e97c827 (config: add `git_die_config()` to the config-set API, 2014-08-07).

Let's have its callers call error() themselves if they want to emit a message, which is exactly what git_die_config() was doing for them before emitting its own die() message.

This means that we can make the vreportf() in usage.c "static", and only expose functions such as usage(), die(), warning() etc.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/fast-import.c |  7 ++++---
 builtin/notes.c       |  6 ++++--
 config.c              | 22 +++++++++-------------
 config.h              | 10 ++++++----
 git-compat-util.h     |  1 -
 imap-send.c           |  3 ++-
 usage.c               |  2 +-
 7 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2b2e28bad79..4e2432bb491 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3456,9 +3456,10 @@ static void git_pack_config(void)
 	}
 	if (!git_config_get_int("pack.indexversion", &indexversion_value)) {
 		pack_idx_opts.version = indexversion_value;
-		if (pack_idx_opts.version > 2)
-			git_die_config("pack.indexversion",
-					"bad pack.indexversion=%"PRIu32, pack_idx_opts.version);
+		if (pack_idx_opts.version > 2) {
+			error("bad pack.indexversion=%"PRIu32, pack_idx_opts.version);
+			git_die_config("pack.indexversion");
+		}
 	}
 	if (!git_config_get_ulong("pack.packsizelimit", &packsizelimit_value))
 		max_packsize = packsizelimit_value;
diff --git a/builtin/notes.c b/builtin/notes.c
index 2812d1eac40..60c5dab4122 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -763,8 +763,10 @@ static int git_config_get_notes_strategy(const char *key,
 
 	if (git_config_get_string(key, &value))
 		return 1;
-	if (parse_notes_merge_strategy(value, strategy))
-		git_die_config(key, _("unknown notes merge strategy %s"), value);
+	if (parse_notes_merge_strategy(value, strategy)) {
+		error(_("unknown notes merge strategy %s"), value);
+		git_die_config(key);
+	}
 
 	free(value);
 	return 0;
diff --git a/config.c b/config.c
index c5873f3a706..30f7971e0cc 100644
--- a/config.c
+++ b/config.c
@@ -2323,7 +2323,7 @@ int repo_config_get_string(struct repository *repo,
 	git_config_check_init(repo);
 	ret = git_configset_get_string(repo->config, key, dest);
 	if (ret < 0)
-		git_die_config(key, NULL);
+		git_die_config(key);
 	return ret;
 }
 
@@ -2334,7 +2334,7 @@ int repo_config_get_string_tmp(struct repository *repo,
 	git_config_check_init(repo);
 	ret = git_configset_get_string_tmp(repo->config, key, dest);
 	if (ret < 0)
-		git_die_config(key, NULL);
+		git_die_config(key);
 	return ret;
 }
 
@@ -2380,7 +2380,7 @@ int repo_config_get_pathname(struct repository *repo,
 	git_config_check_init(repo);
 	ret = git_configset_get_pathname(repo->config, key, dest);
 	if (ret < 0)
-		git_die_config(key, NULL);
+		git_die_config(key);
 	return ret;
 }
 
@@ -2452,8 +2452,10 @@ int git_config_get_expiry(const char *key, const char **output)
 		return ret;
 	if (strcmp(*output, "now")) {
 		timestamp_t now = approxidate("now");
-		if (approxidate(*output) >= now)
-			git_die_config(key, _("Invalid %s: '%s'"), key, *output);
+		if (approxidate(*output) >= now) {
+			error(_("Invalid %s: '%s'"), key, *output);
+			git_die_config(key);
+		}
 	}
 	return ret;
 }
@@ -2550,18 +2552,12 @@ void git_die_config_linenr(const char *key, const char *filename, int linenr)
 		    key, filename, linenr);
 }
 
-NORETURN __attribute__((format(printf, 2, 3)))
-void git_die_config(const char *key, const char *err, ...)
+NORETURN
+void git_die_config(const char *key)
 {
 	const struct string_list *values;
 	struct key_value_info *kv_info;
 
-	if (err) {
-		va_list params;
-		va_start(params, err);
-		vreportf("error: ", err, params);
-		va_end(params);
-	}
 	values = git_config_get_value_multi(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 f119de01309..fae585d2005 100644
--- a/config.h
+++ b/config.h
@@ -626,11 +626,13 @@ struct key_value_info {
 };
 
 /**
- * First prints the error message specified by the caller in `err` and then
- * dies printing the line number and the file name of the highest priority
- * value for the configuration variable `key`.
+ * Dies printing the line number and the file name of the highest
+ * priority value for the configuration variable `key`.
+ *
+ * Consider calling error() first with a more specific formatted
+ * message of your own.
  */
-NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));
+NORETURN void git_die_config(const char *key);
 
 /**
  * Helper function which formats the die error message according to the
diff --git a/git-compat-util.h b/git-compat-util.h
index c6c6f7d6b51..bdb3977b9ec 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -474,7 +474,6 @@ static inline int git_has_dir_sep(const char *path)
 struct strbuf;
 
 /* General helper functions */
-void vreportf(const char *prefix, const char *err, va_list params);
 NORETURN void usage(const char *err);
 NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
 NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
diff --git a/imap-send.c b/imap-send.c
index e6090a0346a..0fdfe5159eb 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1350,7 +1350,8 @@ static int git_imap_config(const char *var, const char *val, void *cb)
 		server.port = git_config_int(var, val);
 	else if (!strcmp("imap.host", var)) {
 		if (!val) {
-			git_die_config("imap.host", "Missing value for 'imap.host'");
+			error("Missing value for 'imap.host'");
+			git_die_config("imap.host");
 		} else {
 			if (starts_with(val, "imap:"))
 				val += 5;
diff --git a/usage.c b/usage.c
index 3d09e8eea48..9943dd8742e 100644
--- a/usage.c
+++ b/usage.c
@@ -6,7 +6,7 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
-void vreportf(const char *prefix, const char *err, va_list params)
+static void vreportf(const char *prefix, const char *err, va_list params)
 {
 	char msg[4096];
 	char *p, *pend = msg + sizeof(msg);

-- 
2.34.1.898.g5a552c2e5f0
Previous: Junio C HamanoNext: Junio C Hamano
Message 7 of 22 in “usage API: Add and use die_message()”
  1. 0/4 usage API: Add and use die_message()Ævar Arnfjörð Bjarmason, Dec 6, 2021
  2. 2/4 usage.c API users: use die_message() where appropriateÆvar Arnfjörð Bjarmason, Dec 6, 2021
  3. Junio C HamanoDec 6, 2021
  4. 1/4 usage.c: add a die_message() routineÆvar Arnfjörð Bjarmason, Dec 6, 2021
  5. Junio C HamanoDec 6, 2021
  6. Junio C HamanoDec 6, 2021
  7. 4/4 config API: don't use vreportf(), make it static in usage.cÆvar Arnfjörð Bjarmason, Dec 6, 2021
  8. Junio C HamanoDec 6, 2021
  9. Ævar Arnfjörð BjarmasonDec 7, 2021
  10. 3/4 usage.c + gc: add and use a die_message_errno()Ævar Arnfjörð Bjarmason, Dec 6, 2021
  11. Junio C HamanoDec 6, 2021
  12. 0/6 usage API: Add and use die_message()Ævar Arnfjörð Bjarmason, Dec 7, 2021
  13. 1/6 usage.c: add a die_message() routineÆvar Arnfjörð Bjarmason, Dec 7, 2021
  14. 2/6 usage.c API users: use die_message() for "fatal :" + exit 128Ævar Arnfjörð Bjarmason, Dec 7, 2021
  15. 3/6 usage.c API users: use die_message() for error() + exit 128Ævar Arnfjörð Bjarmason, Dec 7, 2021
  16. 4/6 gc: return from cmd_gc(), don't call exit()Ævar Arnfjörð Bjarmason, Dec 7, 2021
  17. 5/6 usage.c + gc: add and use a die_message_errno()Ævar Arnfjörð Bjarmason, Dec 7, 2021
  18. 6/6 config API: use get_error_routine(), not vreportf()Ævar Arnfjörð Bjarmason, Dec 7, 2021
  19. Junio C HamanoDec 7, 2021
  20. Jonathan TanDec 22, 2021
  21. Junio C HamanoDec 22, 2021
  22. Ævar Arnfjörð BjarmasonDec 24, 2021

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.