From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 28 Sep 2020 07:16:25 GMT
Subject: Re: [PATCH v2 4/7] worktree: inline `worktree_ref()` into its only caller
Message-ID: <CAPig+cS3x1PtNeiMU5a7Dfa8-cCvs4K=LRwELEcb3DQ+ahfeWA@mail.gmail.com>
In-Reply-To: <CAN0heSr0mZDpjJysvzeG-et3uxpzDWcP3AdPVxiE+KYeY-7nQA@mail.gmail.com>

On Mon, Sep 28, 2020 at 2:57 AM Martin Ågren <martin.agren@gmail.com> wrote:
> On Mon, 28 Sep 2020 at 07:30, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > If I understand correctly, this strbuf_reset() -- which, I suppose,
> > moved here from the retired worktree_ref() -- is no longer needed now
> > that the strbuf stopped being static. So, this line should be dropped
> > from the patch.
>
> What's not obvious from the diff is that this happens inside a loop
> where we go through all worktrees. The strbuf could live one indentation
> level deeper, in which case we'd continuously initialize and release it.
> I placed it at the function-level instead, so that we initialize it
> once and release it once. The "cost" for that is this reset call.
>
> So it's sort of the same reset as before this patch, but it's local to
> this function.

Yep, ignore my stupid comment. I was reading both the patch and the
code yet the reset-in-loop still failed to register.