Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Dec 2, 2021 at 4:07 AM Anders Kaseorg <andersk@mit.edu> wrote:
> On Wed, 1 Dec 2021, Eric Sunshine wrote:
> > If we instead hoist ownership of `worktrees` up to execute_commands()
> > -- which calls execute_commands_atomic() or
> > execute_commands_non_atomic() -- then we can get by with retrieving
> > the worktrees just once, and all those noise changes in update() can
> > be dropped since it will no longer be responsible for allocating or
> > freeing `worktrees`.
>
> Seems reasonable to me.  It’s a smaller textual change, potentially a
> larger conceptual change, but the efficiency improvement is probably
> worthwhile.
>
> That would modify patch 4 to the below.  Patches 5 through 8 cleanly
> rebase past this modification.

Yes, this version looks good. Thanks for accommodating my suggestion.
Not only does it make for a less noisy diff (which is pleasing), but
perhaps more importantly, it reduces cognitive load a small amount (at
least for me[1]).

> Subject: [PATCH v7½ 4/8] worktree: simplify find_shared_symref() memory ownership model

I suspect that Junio will find it easier to take a full re-roll than
dealing with a single patch replacement since he doesn't necessarily
follow these discussions closely. It's more likely that he'll pick up
this change if you post v8 in its entirety.

[1]: Although we regularly use `goto` for this sort of resource
cleanup, most such functions are much shorter than the one here;
somehow the length of this function made the new `goto`s a tiny bit
less easy to grasp-at-a-glance. Purely subjective, I know.