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

[karthik nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2124 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

[snip]

> 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.

> -		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]

> @@ -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.

> -	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]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]