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

[PATCH 15/18] fill_sha1_file: write into a strbuf

From
Jeff King <peff@peff.net>
Date
Oct 3, 2016, 20:36 UTC
Message-ID
<20161003203609.4hig3e24lyvswdcf@sigill.intra.peff.net>
In-Reply-To
<20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>

It's currently the responsibility of the caller to give fill_sha1_file() enough bytes to write into, leading them to manually compute the required lengths. Instead, let's just write into a strbuf so that it's impossible to get this wrong.

The alt_odb caller already has a strbuf, so this makes things strictly simpler. The other caller, sha1_file_name(), uses a static PATH_MAX buffer and dies when it would overflow. We can convert this to a static strbuf, which means our allocation cost is amortized (and as a bonus, we no longer have to worry about PATH_MAX being too short for normal use).

This does introduce some small overhead in fill_sha1_file(), as each strbuf_addchar() will check whether it needs to grow. However, between the optimization in fec501d (strbuf_addch: avoid calling strbuf_grow, 2015-04-16) and the fact that this is not generally called in a tight loop (after all, the next step is typically to access the file!) this probably doesn't matter. And even if it did, the right place to micro-optimize is inside fill_sha1_file(), by calling a single strbuf_grow() there.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1_file.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index efc8cee..80a3333 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -172,36 +172,28 @@ enum scld_error safe_create_leading_directories_const(const char *path)
 	return result;
 }
 
-static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
+static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
 {
 	int i;
 	for (i = 0; i < 20; i++) {
 		static char hex[] = "0123456789abcdef";
 		unsigned int val = sha1[i];
-		*pathbuf++ = hex[val >> 4];
-		*pathbuf++ = hex[val & 0xf];
+		strbuf_addch(buf, hex[val >> 4]);
+		strbuf_addch(buf, hex[val & 0xf]);
 		if (!i)
-			*pathbuf++ = '/';
+			strbuf_addch(buf, '/');
 	}
-	*pathbuf = '\0';
 }
 
 const char *sha1_file_name(const unsigned char *sha1)
 {
-	static char buf[PATH_MAX];
-	const char *objdir;
-	int len;
+	static struct strbuf buf = STRBUF_INIT;
 
-	objdir = get_object_directory();
-	len = strlen(objdir);
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "%s/", get_object_directory());
 
-	/* '/' + sha1(2) + '/' + sha1(38) + '\0' */
-	if (len + 43 > PATH_MAX)
-		die("insanely long object directory %s", objdir);
-	memcpy(buf, objdir, len);
-	buf[len] = '/';
-	fill_sha1_path(buf + len + 1, sha1);
-	return buf;
+	fill_sha1_path(&buf, sha1);
+	return buf.buf;
 }
 
 struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
@@ -213,14 +205,8 @@ struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
 static const char *alt_sha1_path(struct alternate_object_database *alt,
 				 const unsigned char *sha1)
 {
-	/* hex sha1 plus internal "/" */
-	size_t len = GIT_SHA1_HEXSZ + 1;
 	struct strbuf *buf = alt_scratch_buf(alt);
-
-	strbuf_grow(buf, len);
-	fill_sha1_path(buf->buf + buf->len, sha1);
-	strbuf_setlen(buf, buf->len + len);
-
+	fill_sha1_path(buf, sha1);
 	return buf->buf;
 }
 
-- 
2.10.0.618.g82cc264
Previous: Jeff KingNext: Jacob Keller
Message 65 of 84 in “alternate object database cleanups”
  1. 0/18 alternate object database cleanupsJeff King, Oct 3, 2016
  2. 01/18 t5613: drop reachable_via functionJeff King, Oct 3, 2016
  3. Jacob KellerOct 4, 2016
  4. Jeff KingOct 4, 2016
  5. 02/18 t5613: drop test_valid_repo functionJeff King, Oct 3, 2016
  6. Jacob KellerOct 4, 2016
  7. 03/18 t5613: use test_must_failJeff King, Oct 3, 2016
  8. Jacob KellerOct 4, 2016
  9. 05/18 t5613: do not chdir in main processJeff King, Oct 3, 2016
  10. Jacob KellerOct 4, 2016
  11. Junio C HamanoOct 4, 2016
  12. 04/18 t5613: whitespace/style cleanupsJeff King, Oct 3, 2016
  13. Jacob KellerOct 4, 2016
  14. Jeff KingOct 4, 2016
  15. Jacob KellerOct 4, 2016
  16. 06/18 t5613: clarify "too deep" recursion testsJeff King, Oct 3, 2016
  17. Jacob KellerOct 4, 2016
  18. Jeff KingOct 4, 2016
  19. Jacob KellerOct 4, 2016
  20. Jeff KingOct 4, 2016
  21. Jacob KellerOct 4, 2016
  22. Jeff KingOct 4, 2016
  23. Stefan BellerOct 4, 2016
  24. Jeff KingOct 4, 2016
  25. Jakub NarębskiOct 5, 2016
  26. Jeff KingOct 5, 2016
  27. Junio C HamanoOct 5, 2016
  28. Jacob KellerOct 5, 2016
  29. Jacob KellerOct 4, 2016
  30. Jeff KingOct 4, 2016
  31. Jacob KellerOct 4, 2016
  32. 07/18 link_alt_odb_entry: handle normalize_path errorsJeff King, Oct 3, 2016
  33. Jacob KellerOct 4, 2016
  34. Junio C HamanoOct 4, 2016
  35. René ScharfeOct 5, 2016
  36. Jeff KingOct 5, 2016
  37. Bryan TurnerNov 7, 2016
  38. Jeff KingNov 8, 2016
  39. Bryan TurnerNov 8, 2016
  40. Jeff KingNov 8, 2016
  41. Bryan TurnerNov 8, 2016
  42. 08/18 link_alt_odb_entry: refactor string handlingJeff King, Oct 3, 2016
  43. Jacob KellerOct 4, 2016
  44. Jeff KingOct 4, 2016
  45. Jacob KellerOct 4, 2016
  46. Junio C HamanoOct 4, 2016
  47. 09/18 alternates: provide helper for adding to alternates listJeff King, Oct 3, 2016
  48. Jacob KellerOct 4, 2016
  49. 10/18 alternates: provide helper for allocating alternateJeff King, Oct 3, 2016
  50. Jacob KellerOct 4, 2016
  51. 11/18 alternates: encapsulate alt->base mungingJeff King, Oct 3, 2016
  52. 12/18 alternates: use a separate scratch spaceJeff King, Oct 3, 2016
  53. Jacob KellerOct 4, 2016
  54. Junio C HamanoOct 4, 2016
  55. Jeff KingOct 4, 2016
  56. Junio C HamanoOct 4, 2016
  57. Jeff KingOct 4, 2016
  58. 13/18 fill_sha1_file: write "boring" charactersJeff King, Oct 3, 2016
  59. Jacob KellerOct 4, 2016
  60. Junio C HamanoOct 4, 2016
  61. Jeff KingOct 4, 2016
  62. Jacob KellerOct 4, 2016
  63. Junio C HamanoOct 5, 2016
  64. 14/18 alternates: store scratch buffer as strbufJeff King, Oct 3, 2016
  65. 15/18 fill_sha1_file: write into a strbufJeff King, Oct 3, 2016
  66. Jacob KellerOct 4, 2016
  67. 16/18 count-objects: report alternates via verbose modeJeff King, Oct 3, 2016
  68. Jacob KellerOct 4, 2016
  69. Jeff KingOct 4, 2016
  70. Jakub NarębskiOct 5, 2016
  71. René ScharfeOct 5, 2016
  72. 17/18 sha1_file: always allow relative paths to alternatesJeff King, Oct 3, 2016
  73. Jacob KellerOct 4, 2016
  74. Jeff KingOct 4, 2016
  75. 18/18 alternates: use fspathcmp to detect duplicatesJeff King, Oct 3, 2016
  76. Jacob KellerOct 4, 2016
  77. Jeff KingOct 4, 2016
  78. Junio C HamanoOct 4, 2016
  79. Aaron SchrabOct 5, 2016
  80. Jeff KingOct 5, 2016
  81. Jacob KellerOct 4, 2016
  82. Jeff KingOct 4, 2016
  83. Jacob KellerOct 4, 2016
  84. René ScharfeOct 5, 2016

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.