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

[PATCH 3/9] imap-send: don't use git_die_config() inside callback

From
Jeff King <peff@peff.net>
Date
Dec 7, 2023, 07:24 UTC
Message-ID
<20231207072458.GC1277973@coredump.intra.peff.net>
In-Reply-To
<20231207072338.GA1277727@coredump.intra.peff.net>

The point of git_die_config() is to let configset users mention the file/line info for invalid config, like:

  if (!git_config_get_int("foo.bar", &value)) {
	if (!is_ok(value))
		git_die_config("foo.bar");
  }

Using it from within a config callback is unnecessary, because we can simply return an error, at which point the config machinery will mention the file/line of the offending variable. Worse, using git_die_config() can actually produce the wrong location when the key is found in multiple spots. For instance, with config like:

  [imap]
  host
  host = foo

we'll report the line number of the "host = foo" line, but the problem is on the implicit-bool "host" line.

We can fix it by just returning an error code.
Signed-off-by: Jeff King <peff@peff.net>
---
 imap-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/imap-send.c b/imap-send.c
index 996651e4f8..5b0fe4f95a 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1346,7 +1346,7 @@ static int git_imap_config(const char *var, const char *val,
 		server.port = git_config_int(var, val, ctx->kvi);
 	else if (!strcmp("imap.host", var)) {
 		if (!val) {
-			git_die_config("imap.host", "Missing value for 'imap.host'");
+			return error("Missing value for 'imap.host'");
 		} else {
 			if (starts_with(val, "imap:"))
 				val += 5;
-- 
2.43.0.664.ga12c899002
Previous: Jeff KingNext: Patrick Steinhardt
Message 5 of 16 in “bonus config cleanups”
  1. 0/9 bonus config cleanupsJeff King, Dec 7, 2023
  2. 1/9 config: reject bogus values for core.checkstatJeff King, Dec 7, 2023
  3. Taylor BlauDec 8, 2023
  4. 2/9 git_xmerge_config(): prefer error() to die()Jeff King, Dec 7, 2023
  5. 3/9 imap-send: don't use git_die_config() inside callbackJeff King, Dec 7, 2023
  6. Patrick SteinhardtDec 7, 2023
  7. Taylor BlauDec 8, 2023
  8. Patrick SteinhardtDec 11, 2023
  9. Jeff KingDec 12, 2023
  10. 4/9 config: use config_error_nonbool() instead of custom messagesJeff King, Dec 7, 2023
  11. 5/9 diff: give more detailed messages for bogus diff.* configJeff King, Dec 7, 2023
  12. 6/9 config: use git_config_string() for core.checkRoundTripEncodingJeff King, Dec 7, 2023
  13. 7/9 push: drop confusing configset/callback redundancyJeff King, Dec 7, 2023
  14. 8/9 gpg-interface: drop pointless config_error_nonbool() checksJeff King, Dec 7, 2023
  15. 9/9 sequencer: simplify away extra git_config_string() callJeff King, Dec 7, 2023
  16. Patrick SteinhardtDec 7, 2023

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.