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

From: Junio C Hamano <[email-masked]>
Date: Thu, 10 Oct 2024 13:03:30 -0700

Caleb White <[email-masked]> writes:

>> If so, why not just return "infer_backlink.len"?
>
> I would say because the purpose of the return is a boolean,
> either the call was successful or it wasn't. The developer
> knowledge that you speak of should be a given---if the
> function returned true then there's obviously a path
> in the strbuf.

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.

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).

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.