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

[PATCH 09/17] refs.c: remove extra git_path calls from read_loose_refs

From
Jeff King <peff@peff.net>
Date
Aug 10, 2015, 09:36 UTC
Message-ID
<20150810093619.GI30981@sigill.intra.peff.net>
In-Reply-To
<20150810092731.GA9027@sigill.intra.peff.net>

In iterating over the loose refs in "refs/foo/", we keep a running strbuf with "refs/foo/one", "refs/foo/two", etc. But we also need to access these files in the filesystem, as ".git/refs/foo/one", etc. For this latter purpose, we make a series of independent calls to git_path(). These are safe (we only use the result to call stat()), but assigning the result of git_path is a suspicious pattern that we'd rather avoid.

This patch keeps a running buffer with ".git/refs/foo/", and we can just append/reset each directory element as we loop. This matches how we handle the refnames. It should also be more efficient, as we do not keep formatting the same ".git/refs/foo" prefix (which can be arbitrarily deep).

Technically we are dropping a call to strbuf_cleanup() on each generated filename, but that's OK; it wasn't doing anything, as we are putting in single-level names we read from the filesystem (so it could not possibly be cleaning up cruft like "./" in this instance).

A clever reader may also note that the running refname buffer ("refs/foo/") is actually a subset of the filesystem path buffer (".git/refs/foo/"). We could get by with one buffer, indexing the length of $GIT_DIR when we want the refname. However, having tried this, the resulting code actually ends up a little more confusing, and the efficiency improvement is tiny (and almost certainly dwarfed by the system calls we are making).

Signed-off-by: Jeff King <peff@peff.net>
---
 refs.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index e70941a..06f95c4 100644
--- a/refs.c
+++ b/refs.c
@@ -1352,19 +1352,23 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
 {
 	struct ref_cache *refs = dir->ref_cache;
 	DIR *d;
-	const char *path;
 	struct dirent *de;
 	int dirnamelen = strlen(dirname);
 	struct strbuf refname;
+	struct strbuf path = STRBUF_INIT;
+	size_t path_baselen;
 
 	if (*refs->name)
-		path = git_path_submodule(refs->name, "%s", dirname);
+		strbuf_git_path_submodule(&path, refs->name, "%s", dirname);
 	else
-		path = git_path("%s", dirname);
+		strbuf_git_path(&path, "%s", dirname);
+	path_baselen = path.len;
 
-	d = opendir(path);
-	if (!d)
+	d = opendir(path.buf);
+	if (!d) {
+		strbuf_release(&path);
 		return;
+	}
 
 	strbuf_init(&refname, dirnamelen + 257);
 	strbuf_add(&refname, dirname, dirnamelen);
@@ -1373,17 +1377,14 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
 		unsigned char sha1[20];
 		struct stat st;
 		int flag;
-		const char *refdir;
 
 		if (de->d_name[0] == '.')
 			continue;
 		if (ends_with(de->d_name, ".lock"))
 			continue;
 		strbuf_addstr(&refname, de->d_name);
-		refdir = *refs->name
-			? git_path_submodule(refs->name, "%s", refname.buf)
-			: git_path("%s", refname.buf);
-		if (stat(refdir, &st) < 0) {
+		strbuf_addstr(&path, de->d_name);
+		if (stat(path.buf, &st) < 0) {
 			; /* silently ignore */
 		} else if (S_ISDIR(st.st_mode)) {
 			strbuf_addch(&refname, '/');
@@ -1430,8 +1431,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
 					 create_ref_entry(refname.buf, sha1, flag, 0));
 		}
 		strbuf_setlen(&refname, dirnamelen);
+		strbuf_setlen(&path, path_baselen);
 	}
 	strbuf_release(&refname);
+	strbuf_release(&path);
 	closedir(d);
 }
 
-- 
2.5.0.414.g670f2a4
Previous: Jeff KingNext: Jeff King
Message 14 of 34 in “removing questionable uses of git_path”
  1. 0/17 removing questionable uses of git_pathJeff King, Aug 10, 2015
  2. 01/17 cache.h: clarify documentation for git_path, et alJeff King, Aug 10, 2015
  3. 02/17 cache.h: complete set of git_path_submodule helpersJeff King, Aug 10, 2015
  4. 03/17 t5700: modernize styleJeff King, Aug 10, 2015
  5. 04/17 add_to_alternates_file: don't add duplicate entriesJeff King, Aug 10, 2015
  6. Michael HaggertyAug 11, 2015
  7. Jeff KingAug 11, 2015
  8. 05/17 remove hold_lock_file_for_appendJeff King, Aug 10, 2015
  9. Junio C HamanoAug 10, 2015
  10. Jeff KingAug 11, 2015
  11. 06/17 prefer git_pathdup to git_path in some possibly-dangerous casesJeff King, Aug 10, 2015
  12. 07/17 prefer mkpathdup to mkpath in assignmentsJeff King, Aug 10, 2015
  13. 08/17 remote.c: drop extraneous local variable from migrate_fileJeff King, Aug 10, 2015
  14. 09/17 refs.c: remove extra git_path calls from read_loose_refsJeff King, Aug 10, 2015
  15. 10/17 path.c: drop git_path_submoduleJeff King, Aug 10, 2015
  16. Junio C HamanoAug 10, 2015
  17. Junio C HamanoAug 10, 2015
  18. Junio C HamanoAug 10, 2015
  19. Jeff KingAug 11, 2015
  20. 11/17 refs.c: simplify strbufs in reflog setup and writingJeff King, Aug 10, 2015
  21. Michael HaggertyAug 10, 2015
  22. Jeff KingAug 10, 2015
  23. 12/17 refs.c: avoid repeated git_path calls in rename_tmp_logJeff King, Aug 10, 2015
  24. 13/17 refs.c: avoid git_path assignment in lock_ref_sha1_basicJeff King, Aug 10, 2015
  25. 14/17 refs.c: remove_empty_directories can take a strbufJeff King, Aug 10, 2015
  26. 15/17 find_hook: keep our own static bufferJeff King, Aug 10, 2015
  27. 16/17 get_repo_path: refactor path-allocationJeff King, Aug 10, 2015
  28. 17/17 memoize common git-path "constant" filesJeff King, Aug 10, 2015
  29. Michael HaggertyAug 10, 2015
  30. Jeff KingAug 10, 2015
  31. Michael HaggertyAug 10, 2015
  32. Junio C HamanoAug 10, 2015
  33. Jeff KingAug 10, 2015
  34. Duy NguyenAug 15, 2015

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.