Patrick Steinhardt [off-list ref] writes:

There are only three callers:

  - "remote.c:ignore_symref_update()" only cares whether the return
    value is 0 or not.

  - "builtin/remote.c:mv()" is the same.

  - "refs.c:migrate_one_ref()" assumes the behaviour of the reftable
    backend and only checks for negative error codes.

So you could argue that it's the "files" backend that is broken, not the
"reftable" backend. Doesn't matter ultimately though, the real problem
is that this wasn't ever documented anywhere.

You're correct that it does not matter ultimately.  But as a general
rule, which also applies here, anything newly introduced one does
differently without having a good reason is a bug in the new thing,
I would say.

Another solution could be to have the comment in "refs.h" for the
caller-facing function, while the backend pointer simply says "Please
refer to the documentation of `refs_read_symbolic_ref()`."

Yup, avoiding unnecessary duplication is a good thing.

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.