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

From: Junio C Hamano <[email-masked]>
Date: Mon, 15 Nov 2021 21:39:08 -0800

Anders Kaseorg <[email-masked]> writes:

> Storing the worktrees list in a static variable meant that
> find_shared_symref() had to rebuild the list on each call (which is
> inefficient when the call site is in a loop), and also that each call
> invalidated the pointer returned by the previous call (which is
> confusing).
>
> Instead, make it the caller’s responsibility to pass in the worktrees
> list and manage its lifetime.

Very nice.

> +	struct worktree **worktrees = get_worktrees();
> +	const struct worktree *worktree =
> +		is_bare_repository() ?
> +			NULL :
> +			find_shared_symref(worktrees, "HEAD", name);
>  
>  	/* only refs/... are allowed */
>  	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
>  		rp_error("refusing to create funny ref '%s' remotely", name);
> -		return "funny refname";
> +		ret = "funny refname";
> +		goto out;
>  	}

Nice touch to make the code clean after itself in a single place.
Good.

> ...
> +out:
> +	free_worktrees(worktrees);
> +	return ret;
>  }