On Tue Nov 19, 2024 at 08:26, Patrick Steinhardt [off-list ref] wrote:

On Tue, Nov 19, 2024 at 03:54:05PM +0900, Junio C Hamano wrote:

quoted

Patrick Steinhardt [off-list ref] writes:

quoted

The reason why I've been proposing to return negative is because we have
the idiom of checking `err < 0` in many places, so a function that
returns a positive value in the case where it didn't return the expected
result can easily lead to bugs.

I agree with the general reasoning.  I am saying this may or may not
be an error, and if it turns out that it is not an error but is just
one of the generally expected outcome, treating it as an error and
having "if (status < 0)" to lump the case together with other error
cases may not be nice to the callers.

The question to me is whether the function returns something sensible in
all non-error cases that a caller can use properly without having to
explicitly check for the value. And I'd say that this is not the case
with `refs_read_symbolic_ref()`, which wouldn't end up setting the value
of `referent`.

So regardless of whether we define this as error or non-error, the
caller would have to exlicitly handle the case where it's not a symref
in order to make sense of it because the result is not well-defined.

I agree with Patrick re the -1, -2 return values. The non-error behavior should
be when referent is set, anything else is something the caller would need to
consider if they want to do something with it or not.