On 20.10.25 17:34, Markus Armbruster wrote:
Daniel P. Berrangé <[email protected]> writes:
On Mon, Oct 20, 2025 at 02:22:22PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 20.10.25 14:05, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <[email protected]> writes:
Recently we moved to returning errp. Why to keep int return value?
Generally it doesn't help: you can't use in a logic of handling
an error, as you are never sure, that in future the logic in
the stack will not change: it may start to return another error
code in the same case, or return same error code in another case.
Actually, we can only rely on concrete errno code when get it
_directly_ from documented library function or syscall. This way we
handle for example EINTR. But later in a stack, we can only add
this errno to the textual error by strerror().
It's a matter of the function's contract, actually.
If the contract is "Return negative value on failure", checking for
failure is all you can do with it. Same information as "Return false on
failure".
If the contract is "Return negative errno on failure", the function is
responsible for returning values that make sense. Ideally, the contract
spells them all out.
Do you know an example in code where we have both errno return value
and errp, and the return value make sense and used by callers?
If there are examples of that, I would generally consider them to be
bugs.
IMHO if a method is using "Error **errp", then it should be considered
forbidden to return 'errno' values.
Several subsystems disagree :)
I'd vote, that in 99% (or more) cases, they don't reasonably disagree,
but blindly follow usual pattern of returning -errno together with
errp, while having no reasonable contract on concrete errno values,
and with this errno finally unused (used only to check, it is it < 0,
like boolean). In other words, the only contract they have is
"< 0 is error, otherwise success".
Quick & dirty search without a claim to accuracy or completeness:
$ git-ls-files \*.[ch] | xargs awk '/, Error \*\*errp/ { on=1 } on && /return -E/ { print
FILENAME ":" FNR ":" $0 } /^}/ { on=0 }'
If there is a need for distinguishing some cases from others, then keep
with int '0/-1' example, but turn it into a multi-value return such as
1/0/-1, or 0/-1/-2/-3/..., etc with named constants for the unusual
scenarios. An example of that would be QIOChannel were we introduced
"#define QIO_CHANNEL_ERR_BLOCK -2" to replace the need for EAGAIN checks
in callers.
Defining your own error codes is fine.
Reusing errno codes can also be fine.
In both cases, the function contract is a load-bearing component.
--
Best regards,
Vladimir
