Re: [PATCH v2 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Dec 11, 2023, 09:36 UTC
- Message-ID
- <CAPig+cTmAqDqu4Hiz+JO2GfV3+CqgVTXwFKWSTJXXAJ8Kg-xbw@mail.gmail.com>
- In-Reply-To
- <ZXbRkOiD80zT7tC5@tanuki>
On Mon, Dec 11, 2023 at 4:08 AM Patrick Steinhardt <ps@pks.im> wrote:
Show 19 quoted lines
> On Fri, Dec 08, 2023 at 06:46:33PM -0500, Eric Sunshine wrote:
> > In this case, though, assuming I understand the intent, I think the
> > more common and safe idiom in this codebase is something like this:
> >
> > struct strbuf name = STRBUF_INIT;
> > strbuf_addstr(&name, "branch");
> > size_t len = name.len;
> > for (...) {
> > strbuf_setlen(&name, len);
> > strbuf_addf(&name, "%04d", i);
> > ref.refname = name.buf;
> > ...
> > }
> > strbuf_release(&name);
>
> Yeah, I'll convert this to use a `struct strbuf` instead. But instead of
> tracking the length I'll just use a `strbuf_reset()` followed by
> `strbuf_addf("branch-%04d")`. It's simpler to read and we don't need to
> squeeze every last drop of performance out of this loop anyway.Sounds perfectly reasonable to me, and I agree with the reasoning, especially the lower cognitive load with strbuf_reset(). Thanks.