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

[PATCH 5/6] pass st.st_size as hint for strbuf_readlink()

From
Jeff King <peff@peff.net>
Date
Jul 24, 2018, 10:51 UTC
Message-ID
<20180724105139.GE17165@sigill.intra.peff.net>
In-Reply-To
<20180724104852.GA14638@sigill.intra.peff.net>

When we initially added the strbuf_readlink() function in b11b7e13f4 (Add generic 'strbuf_readlink()' helper function, 2008-12-17), the point was that we generally have a _guess_ as to the correct size based on the stat information, but we can't necessarily trust it.

Over the years, a few callers have grown up that simply pass in 0, even though they have the stat information. Let's have them pass in their hint for consistency (and in theory efficiency, since it may avoid an extra resize/syscall loop, but neither location is probably performance critical).

Note that st.st_size is actually an off_t, so in theory we need xsize_t() here. But none of the other callsites use it, and since this is just a hint, it doesn't matter either way (if we wrap we'll simply start with a too-small hint and then eventually complain when we cannot allocate the memory).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/init-db.c    | 3 ++-
 refs/files-backend.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 4ecf909368..12ddda7e7b 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -73,7 +73,8 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 			continue;
 		else if (S_ISLNK(st_template.st_mode)) {
 			struct strbuf lnk = STRBUF_INIT;
-			if (strbuf_readlink(&lnk, template_path->buf, 0) < 0)
+			if (strbuf_readlink(&lnk, template_path->buf,
+					    st_template.st_size) < 0)
 				die_errno(_("cannot readlink '%s'"), template_path->buf);
 			if (symlink(lnk.buf, path->buf))
 				die_errno(_("cannot symlink '%s' '%s'"),
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a9a066dcfb..c110c2520c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -363,7 +363,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 	/* Follow "normalized" - ie "refs/.." symlinks by hand */
 	if (S_ISLNK(st.st_mode)) {
 		strbuf_reset(&sb_contents);
-		if (strbuf_readlink(&sb_contents, path, 0) < 0) {
+		if (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {
 			if (errno == ENOENT || errno == EINVAL)
 				/* inconsistent with lstat; retry */
 				goto stat_ref;
-- 
2.18.0.542.g2bf2fc4f7e
Previous: Jeff KingNext: Torsten Bögershausen
Message 6 of 9 in “use size_t in iconv/strbuf”
  1. 0/6 use size_t in iconv/strbufJeff King, Jul 24, 2018
  2. 1/6 reencode_string: use st_add/st_mult helpersJeff King, Jul 24, 2018
  3. 2/6 reencode_string: use size_t for string lengthsJeff King, Jul 24, 2018
  4. 3/6 strbuf: use size_t for length in intermediate variablesJeff King, Jul 24, 2018
  5. 4/6 strbuf_readlink: use ssize_tJeff King, Jul 24, 2018
  6. 5/6 pass st.st_size as hint for strbuf_readlink()Jeff King, Jul 24, 2018
  7. Torsten BögershausenJul 25, 2018
  8. Jeff KingJul 26, 2018
  9. 6/6 strbuf_humanise: use unsigned variablesJeff King, Jul 24, 2018

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.