Re: [PATCH v3 1/3] worktree: refactor infer_backlink() to use *strbuf

From: Caleb White <cdwhite3@pm.me>
Date: Oct 10, 2024, 20:24 UTC
Message-ID: <XUb4_NFTymdqM_NMxxIfI3YZdQXYu0zViporhB_GbnDFY1pjJNaM8usiQB68mu4RIM_ud9bt8lqq28fdeJhYs-J09YKma67O8kuC8rOGG-8=@pm.me>
In-Reply-To: <xmqqcyk7k9nx.fsf@gitster.g>

On Thursday, October 10th, 2024 at 15:03, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

> That does not say what should be left in the strbuf when the request
> failed. If it is undefined, then using its .len (or .buf) is not
> quite right, without relying on too much implementation details of
> the called function.
>

Show 5 quoted lines

> Usually, this project uses 0 to signal a success, while errors are
> signalled by returning a negative value (and if there can be
> multiple ways to fail, such a design leaves different negative
> values as a possibility).
>

Show 6 quoted lines

> If the result the caller finds useful is always positive value (or
> non-negative value), however, it is perfectly fine and often more
> convenient if you used the "positive (or non-negative) value gives
> the intended result and signals a success, negative (or
> non-positive) value signals an error" convention. The caller can
>

> if (0 < do_the_thing())
> ; /* success /
>

> just fine, instead of the boolean "0 is success, negative values are
> various errors",
>

> if (!do_the_thing())
> ; / success */
>

> when it does not care how/why the thing failed.

That makes sense. I can update the function to return -1 on error and the strbuf.len on success.