Re: [PATCH 02/10] reftable: stop using `strbuf_addf()`
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Oct 11, 2024, 09:51 UTC
- Message-ID
- <CAOLa=ZTHuWstTD56ZVTecW7ThHhpEqmrL28Emt5DtPL6pYhcpw@mail.gmail.com>
- In-Reply-To
- <abc28d7664f151e00568a6a3d18bf8a2de46470d.1728629612.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 24 quoted lines
> 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);Here, it is okay to remove this, since we define our own buf array.
Show 8 quoted lines
> - 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)
[snip]
Show 5 quoted lines
> @@ -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);
However here it is different, since we still use the strbuf. I guess it should be okay, since 'buf' is initialized using 'STRBUF_INIT' and that still keeps the buf.len to 0.
Show 7 quoted lines
> - 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); >
So when we do 'strbuf_addstr(&buf, ...)' it should allocate the required memory. But the reset removal did catch my eye.
[snip]