Re: [PATCH v13 2/9] refs: standardize output of refs_read_symbolic_ref
- From
Bence Ferdinandy <bence@ferdinandy.com>
- Date
- Nov 19, 2024, 10:10 UTC
- Message-ID
- <D5Q2NU9LJIVC.23QKQ2GWHUVFQ@ferdinandy.com>
- In-Reply-To
- <Zzw9iqf5bcj5_lV8@pks.im>
On Tue Nov 19, 2024 at 08:26, Patrick Steinhardt <ps@pks.im> wrote:
Show 22 quoted lines
> On Tue, Nov 19, 2024 at 03:54:05PM +0900, Junio C Hamano wrote: >> Patrick Steinhardt <ps@pks.im> writes: >> > 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.