Re: [PATCH] migration: vmsd errp handlers: return bool

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 20.10.25 17:34, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> 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 <vsementsov@yandex-team.ru> 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".

Functions that could just as well return -1 instead of errno exist.

Functions that return negative errno with callers that use them also
exist.

I'm not going to speculate on relative frequency.

I much prefer written function contracts.  But if a caller relies on
negative errno codes, there is an unwritten contract whether we like it
or not.

>> 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 }'

[...]