reftable: stop using `strbuf_addf()`

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@pks.im>
Signed-off-by: Taylor Blau <me@ttaylorr.com>

diff --git a/reftable/stack.c b/reftable/stack.c
index 7e617c25914b2faac3990747036eaa47bf729b99..d7bc1187dfbb5d21157dc9a4f6a841df16726207 100644 (file)
--- 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 d470060e8be24d19a6422bc2afb91df94260a982..8077bbc5e7a910579b70fd4a93a8223531f6296f 100644 (file)
--- 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 27ce84445e821e0d27affe8b5896a54d7a3f95c8..5f59b0ad6ad22a7520d8c4b4c860a31575730304 100644 (file)
--- 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 874095b9ee23457188f09f58c1cf088ad9c11601..b56ea77431271c795ef7c0ec576dbbfe4e2d9fbd 100644 (file)
--- 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);