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

[PATCH 02/10] reftable: stop using `strbuf_addf()`

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 11, 2024, 06:54 UTC
Message-ID
<abc28d7664f151e00568a6a3d18bf8a2de46470d.1728629612.git.ps@pks.im>
In-Reply-To
<cover.1728629612.git.ps@pks.im>

We're about to introduce our own `reftable_buf` type to replace `strbuf`. Get rid of the seldomly-used `strbuf_addf()` function such that we have to reimplement one less function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c                    | 18 ++++++++-----
 t/unit-tests/t-reftable-block.c     |  7 +++--
 t/unit-tests/t-reftable-readwrite.c | 20 +++++++-------
 t/unit-tests/t-reftable-stack.c     | 42 ++++++++++++++++-------------
 4 files changed, 50 insertions(+), 37 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index 7e617c25914..d7bc1187dfb 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1387,12 +1387,18 @@ static int stack_compact_range(struct reftable_stack *st,
 	 * have just written. In case the compacted table became empty we
 	 * simply skip writing it.
 	 */
-	for (i = 0; i < first_to_replace; i++)
-		strbuf_addf(&tables_list_buf, "%s\n", names[i]);
-	if (!is_empty_table)
-		strbuf_addf(&tables_list_buf, "%s\n", new_table_name.buf);
-	for (i = last_to_replace + 1; names[i]; i++)
-		strbuf_addf(&tables_list_buf, "%s\n", names[i]);
+	for (i = 0; i < first_to_replace; i++) {
+		strbuf_addstr(&tables_list_buf, names[i]);
+		strbuf_addstr(&tables_list_buf, "\n");
+	}
+	if (!is_empty_table) {
+		strbuf_addstr(&tables_list_buf, new_table_name.buf);
+		strbuf_addstr(&tables_list_buf, "\n");
+	}
+	for (i = last_to_replace + 1; names[i]; i++) {
+		strbuf_addstr(&tables_list_buf, names[i]);
+		strbuf_addstr(&tables_list_buf, "\n");
+	}
 
 	err = write_in_full(get_lock_file_fd(&tables_list_lock),
 			    tables_list_buf.buf, tables_list_buf.len);
diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c
index d470060e8be..8077bbc5e7a 100644
--- a/t/unit-tests/t-reftable-block.c
+++ b/t/unit-tests/t-reftable-block.c
@@ -308,10 +308,13 @@ static void t_index_block_read_write(void)
 	check(!ret);
 
 	for (i = 0; i < N; i++) {
-		strbuf_init(&recs[i].u.idx.last_key, 9);
+		char buf[128];
+
+		snprintf(buf, sizeof(buf), "branch%02"PRIuMAX, (uintmax_t)i);
 
+		strbuf_init(&recs[i].u.idx.last_key, 9);
 		recs[i].type = BLOCK_TYPE_INDEX;
-		strbuf_addf(&recs[i].u.idx.last_key, "branch%02"PRIuMAX, (uintmax_t)i);
+		strbuf_addstr(&recs[i].u.idx.last_key, buf);
 		recs[i].u.idx.offset = i;
 
 		ret = block_writer_add(&bw, &recs[i]);
diff --git a/t/unit-tests/t-reftable-readwrite.c b/t/unit-tests/t-reftable-readwrite.c
index 27ce84445e8..5f59b0ad6ad 100644
--- a/t/unit-tests/t-reftable-readwrite.c
+++ b/t/unit-tests/t-reftable-readwrite.c
@@ -753,12 +753,13 @@ static void t_write_multiple_indices(void)
 	struct reftable_write_options opts = {
 		.block_size = 100,
 	};
-	struct strbuf writer_buf = STRBUF_INIT, buf = STRBUF_INIT;
+	struct strbuf writer_buf = STRBUF_INIT;
 	struct reftable_block_source source = { 0 };
 	struct reftable_iterator it = { 0 };
 	const struct reftable_stats *stats;
 	struct reftable_writer *writer;
 	struct reftable_reader *reader;
+	char buf[128];
 	int err, i;
 
 	writer = t_reftable_strbuf_writer(&writer_buf, &opts);
@@ -770,9 +771,8 @@ static void t_write_multiple_indices(void)
 			.value.val1 = {i},
 		};
 
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "refs/heads/%04d", i);
-		ref.refname = buf.buf,
+		snprintf(buf, sizeof(buf), "refs/heads/%04d", i);
+		ref.refname = buf;
 
 		err = reftable_writer_add_ref(writer, &ref);
 		check(!err);
@@ -788,9 +788,8 @@ static void t_write_multiple_indices(void)
 			},
 		};
 
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "refs/heads/%04d", i);
-		log.refname = buf.buf,
+		snprintf(buf, sizeof(buf), "refs/heads/%04d", i);
+		log.refname = buf;
 
 		err = reftable_writer_add_log(writer, &log);
 		check(!err);
@@ -824,7 +823,6 @@ static void t_write_multiple_indices(void)
 	reftable_writer_free(writer);
 	reftable_reader_decref(reader);
 	strbuf_release(&writer_buf);
-	strbuf_release(&buf);
 }
 
 static void t_write_multi_level_index(void)
@@ -848,10 +846,10 @@ static void t_write_multi_level_index(void)
 			.value_type = REFTABLE_REF_VAL1,
 			.value.val1 = {i},
 		};
+		char buf[128];
 
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "refs/heads/%03" PRIuMAX, (uintmax_t)i);
-		ref.refname = buf.buf,
+		snprintf(buf, sizeof(buf), "refs/heads/%03" PRIuMAX, (uintmax_t)i);
+		ref.refname = buf;
 
 		err = reftable_writer_add_ref(writer, &ref);
 		check(!err);
diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
index 874095b9ee2..b56ea774312 100644
--- a/t/unit-tests/t-reftable-stack.c
+++ b/t/unit-tests/t-reftable-stack.c
@@ -105,7 +105,6 @@ static int write_test_ref(struct reftable_writer *wr, void *arg)
 static void write_n_ref_tables(struct reftable_stack *st,
 			       size_t n)
 {
-	struct strbuf buf = STRBUF_INIT;
 	int disable_auto_compact;
 	int err;
 
@@ -117,10 +116,10 @@ static void write_n_ref_tables(struct reftable_stack *st,
 			.update_index = reftable_stack_next_update_index(st),
 			.value_type = REFTABLE_REF_VAL1,
 		};
+		char buf[128];
 
-		strbuf_reset(&buf);
-		strbuf_addf(&buf, "refs/heads/branch-%04"PRIuMAX, (uintmax_t)i);
-		ref.refname = buf.buf;
+		snprintf(buf, sizeof(buf), "refs/heads/branch-%04"PRIuMAX, (uintmax_t)i);
+		ref.refname = buf;
 		t_reftable_set_hash(ref.value.val1, i, GIT_SHA1_FORMAT_ID);
 
 		err = reftable_stack_add(st, &write_test_ref, &ref);
@@ -128,7 +127,6 @@ static void write_n_ref_tables(struct reftable_stack *st,
 	}
 
 	st->opts.disable_auto_compact = disable_auto_compact;
-	strbuf_release(&buf);
 }
 
 struct write_log_arg {
@@ -434,7 +432,10 @@ static void t_reftable_stack_auto_compaction_fails_gracefully(void)
 	 * Adding a new table to the stack should not be impacted by this, even
 	 * though auto-compaction will now fail.
 	 */
-	strbuf_addf(&table_path, "%s/%s.lock", dir, st->readers[0]->name);
+	strbuf_addstr(&table_path, dir);
+	strbuf_addstr(&table_path, "/");
+	strbuf_addstr(&table_path, st->readers[0]->name);
+	strbuf_addstr(&table_path, ".lock");
 	write_file_buf(table_path.buf, "", 0);
 
 	ref.update_index = 2;
@@ -1077,8 +1078,10 @@ static void t_reftable_stack_auto_compaction_with_locked_tables(void)
 	 * size, we expect that auto-compaction will want to compact all of the
 	 * tables. Locking any of the tables will keep it from doing so.
 	 */
-	strbuf_reset(&buf);
-	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[2]->name);
+	strbuf_addstr(&buf, dir);
+	strbuf_addstr(&buf, "/");
+	strbuf_addstr(&buf, st->readers[2]->name);
+	strbuf_addstr(&buf, ".lock");
 	write_file_buf(buf.buf, "", 0);
 
 	/*
@@ -1101,7 +1104,6 @@ static void t_reftable_stack_add_performs_auto_compaction(void)
 {
 	struct reftable_write_options opts = { 0 };
 	struct reftable_stack *st = NULL;
-	struct strbuf refname = STRBUF_INIT;
 	char *dir = get_tmp_dir(__LINE__);
 	int err;
 	size_t i, n = 20;
@@ -1115,6 +1117,7 @@ static void t_reftable_stack_add_performs_auto_compaction(void)
 			.value_type = REFTABLE_REF_SYMREF,
 			.value.symref = (char *) "master",
 		};
+		char buf[128];
 
 		/*
 		 * Disable auto-compaction for all but the last runs. Like this
@@ -1123,9 +1126,8 @@ static void t_reftable_stack_add_performs_auto_compaction(void)
 		 */
 		st->opts.disable_auto_compact = i != n;
 
-		strbuf_reset(&refname);
-		strbuf_addf(&refname, "branch-%04"PRIuMAX, (uintmax_t)i);
-		ref.refname = refname.buf;
+		snprintf(buf, sizeof(buf), "branch-%04"PRIuMAX, (uintmax_t)i);
+		ref.refname = buf;
 
 		err = reftable_stack_add(st, write_test_ref, &ref);
 		check(!err);
@@ -1142,7 +1144,6 @@ static void t_reftable_stack_add_performs_auto_compaction(void)
 	}
 
 	reftable_stack_destroy(st);
-	strbuf_release(&refname);
 	clear_dir(dir);
 }
 
@@ -1163,8 +1164,10 @@ static void t_reftable_stack_compaction_with_locked_tables(void)
 	check_int(st->merged->readers_len, ==, 3);
 
 	/* Lock one of the tables that we're about to compact. */
-	strbuf_reset(&buf);
-	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[1]->name);
+	strbuf_addstr(&buf, dir);
+	strbuf_addstr(&buf, "/");
+	strbuf_addstr(&buf, st->readers[1]->name);
+	strbuf_addstr(&buf, ".lock");
 	write_file_buf(buf.buf, "", 0);
 
 	/*
@@ -1321,10 +1324,13 @@ static void t_reftable_stack_reload_with_missing_table(void)
 	 * our old readers. This should trigger a partial reload of the stack,
 	 * where we try to reuse our old readers.
 	*/
-	strbuf_addf(&content, "%s\n", st->readers[0]->name);
-	strbuf_addf(&content, "%s\n", st->readers[1]->name);
+	strbuf_addstr(&content, st->readers[0]->name);
+	strbuf_addstr(&content, "\n");
+	strbuf_addstr(&content, st->readers[1]->name);
+	strbuf_addstr(&content, "\n");
 	strbuf_addstr(&content, "garbage\n");
-	strbuf_addf(&table_path, "%s.lock", st->list_file);
+	strbuf_addstr(&table_path, st->list_file);
+	strbuf_addstr(&table_path, ".lock");
 	write_file_buf(table_path.buf, content.buf, content.len);
 	err = rename(table_path.buf, st->list_file);
 	check(!err);
-- 
2.47.0.dirty
Previous: Patrick SteinhardtNext: karthik nayak
Message 3 of 63 in “reftable: stop using `struct strbuf`”
  1. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 11, 2024
  2. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 11, 2024
  3. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 11, 2024
  4. karthik nayakOct 11, 2024
  5. Patrick SteinhardtOct 14, 2024
  6. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 11, 2024
  7. karthik nayakOct 11, 2024
  8. Patrick SteinhardtOct 14, 2024
  9. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 11, 2024
  10. karthik nayakOct 11, 2024
  11. Patrick SteinhardtOct 14, 2024
  12. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 11, 2024
  13. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 11, 2024
  14. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  15. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  16. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  17. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 11, 2024
  18. karthik nayakOct 11, 2024
  19. Patrick SteinhardtOct 14, 2024
  20. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 14, 2024
  21. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 14, 2024
  22. Taylor BlauOct 14, 2024
  23. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 14, 2024
  24. Taylor BlauOct 14, 2024
  25. Patrick SteinhardtOct 15, 2024
  26. Taylor BlauOct 15, 2024
  27. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 14, 2024
  28. Taylor BlauOct 14, 2024
  29. Patrick SteinhardtOct 15, 2024
  30. Eric SunshineOct 15, 2024
  31. Taylor BlauOct 15, 2024
  32. Patrick SteinhardtOct 16, 2024
  33. Taylor BlauOct 16, 2024
  34. Patrick SteinhardtOct 17, 2024
  35. Taylor BlauOct 17, 2024
  36. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 14, 2024
  37. Taylor BlauOct 14, 2024
  38. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 14, 2024
  39. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 14, 2024
  40. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  41. Taylor BlauOct 14, 2024
  42. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  43. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  44. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 14, 2024
  45. Taylor BlauOct 14, 2024
  46. Patrick SteinhardtOct 15, 2024
  47. shejialuoOct 15, 2024
  48. Patrick SteinhardtOct 15, 2024
  49. shejialuoOct 15, 2024
  50. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 17, 2024
  51. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 17, 2024
  52. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 17, 2024
  53. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 17, 2024
  54. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 17, 2024
  55. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 17, 2024
  56. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 17, 2024
  57. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  58. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  59. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  60. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 17, 2024
  61. Taylor BlauOct 17, 2024
  62. karthik nayakOct 18, 2024
  63. Taylor BlauOct 18, 2024

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.