We're about to introduce our own `reftable_buf` type to replace
`strbuf`. One function we'll have to convert is `strbuf_addf()`, which
is used in a handful of places. This function uses `snprintf()`
internally, which makes porting it a bit more involved:
- It is not available on all platforms.
- Some platforms like Windows have broken implementations.
So by using `snprintf()` we'd also push the burden on downstream users
of the reftable library to make available a properly working version of
it.
Most callsites of `strbuf_addf()` are trivial to convert to not using
it. We do end up using `snprintf()` in our unit tests, but that isn't
much of a problem for downstream users of the reftable library.
While at it, remove a useless call to `strbuf_reset()` in
`t_reftable_stack_auto_compaction_with_locked_tables()`. We don't write
to the buffer before this and initialize it with `STRBUF_INIT`, so there
is no need to reset anything.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
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.72.gef8ce8f3d4.dirty
