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

[PATCH 4/6] userdiff: drop parse_driver function

From
Jeff King <peff@peff.net>
Date
Jan 14, 2013, 15:04 UTC
Message-ID
<20130114150414.GD16828@sigill.intra.peff.net>
In-Reply-To
<20130114145845.GA16497@sigill.intra.peff.net>
When we parse userdiff config, we generally assume that
  diff.name.key

will affect the "key" value of the "name" driver. However, without checking the key, we conflict with the ancient "diff.color.*" namespace. The current code is careful not to even create a driver struct if we do not see a key that is known by the diff-driver code.

However, this carefulness is unnecessary; the default driver with no keys set behaves exactly the same as having no driver at all. We can simply set up the driver struct as soon as we see we have a config key that looks like a driver. This makes the code a bit more readable.

Signed-off-by: Jeff King <peff@peff.net>
---
This is not strictly related to the series, but I noticed it as a
cleanup while doing the previous patch.
 userdiff.c | 50 +++++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)
diff --git a/userdiff.c b/userdiff.c
index 1a6a0fa..c6cdec4 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -184,28 +184,6 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *k, int len)
 	return NULL;
 }
 
-static struct userdiff_driver *parse_driver(const char *var,
-		const char *value, const char *type)
-{
-	struct userdiff_driver *drv;
-	const char *name, *key;
-	int namelen;
-
-	if (match_config_key(var, "diff", &name, &namelen, &key) < 0 ||
-	    strcmp(type, key))
-		return NULL;
-
-	drv = userdiff_find_by_namelen(name, namelen);
-	if (!drv) {
-		ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
-		drv = &drivers[ndrivers++];
-		memset(drv, 0, sizeof(*drv));
-		drv->name = xmemdupz(name, namelen);
-		drv->binary = -1;
-	}
-	return drv;
-}
-
 static int parse_funcname(struct userdiff_funcname *f, const char *k,
 		const char *v, int cflags)
 {
@@ -233,20 +211,34 @@ int userdiff_config(const char *k, const char *v)
 int userdiff_config(const char *k, const char *v)
 {
 	struct userdiff_driver *drv;
+	const char *name, *type;
+	int namelen;
+
+	if (match_config_key(k, "diff", &name, &namelen, &type) || !name)
+		return 0;
+
+	drv = userdiff_find_by_namelen(name, namelen);
+	if (!drv) {
+		ALLOC_GROW(drivers, ndrivers+1, drivers_alloc);
+		drv = &drivers[ndrivers++];
+		memset(drv, 0, sizeof(*drv));
+		drv->name = xmemdupz(name, namelen);
+		drv->binary = -1;
+	}
 
-	if ((drv = parse_driver(k, v, "funcname")))
+	if (!strcmp(type, "funcname"))
 		return parse_funcname(&drv->funcname, k, v, 0);
-	if ((drv = parse_driver(k, v, "xfuncname")))
+	if (!strcmp(type, "xfuncname"))
 		return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
-	if ((drv = parse_driver(k, v, "binary")))
+	if (!strcmp(type, "binary"))
 		return parse_tristate(&drv->binary, k, v);
-	if ((drv = parse_driver(k, v, "command")))
+	if (!strcmp(type, "command"))
 		return git_config_string(&drv->external, k, v);
-	if ((drv = parse_driver(k, v, "textconv")))
+	if (!strcmp(type, "textconv"))
 		return git_config_string(&drv->textconv, k, v);
-	if ((drv = parse_driver(k, v, "cachetextconv")))
+	if (!strcmp(type, "cachetextconv"))
 		return parse_bool(&drv->textconv_want_cache, k, v);
-	if ((drv = parse_driver(k, v, "wordregex")))
+	if (!strcmp(type, "wordregex"))
 		return git_config_string(&drv->word_regex, k, v);
 
 	return 0;
-- 
1.8.1.rc1.10.g7d71f7b
Previous: Jeff KingNext: Jeff King
Message 29 of 31 in “archive-tar: fix sanity check in config parsing”
  1. archive-tar: fix sanity check in config parsingRené Scharfe, Jan 13, 2013
  2. Jeff KingJan 13, 2013
  3. Joachim SchmitzJan 14, 2013
  4. Jeff KingJan 14, 2013
  5. Jeff KingJan 14, 2013
  6. 1/6 config: add helper function for parsing key namesJeff King, Jan 14, 2013
  7. Junio C HamanoJan 14, 2013
  8. Jeff KingJan 15, 2013
  9. Junio C HamanoJan 15, 2013
  10. Junio C HamanoJan 18, 2013
  11. 0/8 config key-parsing cleanupsJeff King, Jan 23, 2013
  12. 1/8 config: add helper function for parsing key namesJeff King, Jan 23, 2013
  13. 2/8 archive-tar: use parse_config_key when parsing configJeff King, Jan 23, 2013
  14. 3/8 convert some config callbacks to parse_config_keyJeff King, Jan 23, 2013
  15. 4/8 userdiff: drop parse_driver functionJeff King, Jan 23, 2013
  16. 5/8 submodule: use parse_config_key when parsing configJeff King, Jan 23, 2013
  17. Jens LehmannJan 23, 2013
  18. 6/8 submodule: simplify memory handling in config parsingJeff King, Jan 23, 2013
  19. Jens LehmannJan 23, 2013
  20. 7/8 help: use parse_config_key for man configJeff King, Jan 23, 2013
  21. 8/8 reflog: use parse_config_key in config callbackJeff King, Jan 23, 2013
  22. Junio C HamanoJan 23, 2013
  23. Jonathan NiederJan 23, 2013
  24. 2/6 archive-tar: use match_config_key when parsing configJeff King, Jan 14, 2013
  25. 3/6 convert some config callbacks to match_config_keyJeff King, Jan 14, 2013
  26. Jonathan NiederJan 14, 2013
  27. Jeff KingJan 14, 2013
  28. Jeff KingJan 14, 2013
  29. 4/6 userdiff: drop parse_driver functionJeff King, Jan 14, 2013
  30. 5/6 submodule: use match_config_key when parsing configJeff King, Jan 14, 2013
  31. 6/6 submodule: simplify memory handling in config parsingJeff King, Jan 14, 2013

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.