Re: [PATCH v4 3/8] worktree: refactor infer_backlink return

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 19, 2024, 15:08 UTC
Message-ID: <977c1e75-1fef-4827-91ae-cc1a6ce7508e@gmail.com>
In-Reply-To: <20241031-wt_relative_options-v4-3-07a3dc0f02a3@pm.me>

Hi Caleb

Thanks for splitting this out into a separate patch

On 01/11/2024 04:38, Caleb White wrote:

Show 7 quoted lines

> -static int infer_backlink(const char *gitfile, struct strbuf *inferred)
> +static ssize_t infer_backlink(const char *gitfile, struct strbuf *inferred)
>   {
>   	struct strbuf actual = STRBUF_INIT;
>   	const char *id;
>   
> +	strbuf_reset(inferred);

I think the code was clearer when we reset the buf just before using it. That way it is easy to see that we add the path to an empty buffer.

Show 21 quoted lines

>   	if (strbuf_read_file(&actual, gitfile, 0) < 0)
>   		goto error;
>   	if (!starts_with(actual.buf, "gitdir:"))
> @@ -741,18 +744,16 @@ static int infer_backlink(const char *gitfile, struct strbuf *inferred)
>   	id++; /* advance past '/' to point at <id> */
>   	if (!*id)
>   		goto error;
> -	strbuf_reset(inferred);
>   	strbuf_git_common_path(inferred, the_repository, "worktrees/%s", id);
>   	if (!is_directory(inferred->buf))
>   		goto error;
>   
>   	strbuf_release(&actual);
> -	return 1;
> -
> +	return inferred->len;
>   error:
>   	strbuf_release(&actual);
>   	strbuf_reset(inferred); /* clear invalid path */
> -	return 0;
> +	return -1;

Why don't we need to update the callers of this function to account for the new return value?

Best Wishes

Phillip