Markus Armbruster <arm...@redhat.com> wrote:
> "Zhijian Li (Fujitsu)" <lizhij...@fujitsu.com> writes:
>
>> On 18/09/2023 22:41, Markus Armbruster wrote:
>>> When a function returns 0 on success, negative value on error,
>>> checking for non-zero suffices, but checking for negative is clearer.
>>> So do that.
>>>
>>
>> This patch is no my favor convention.
>
> Certainly a matter of taste, which means maintainers get to decide, not
> me.
>
> Failure checks can be confusing in C. Is
>
> if (foo(...))
>
> checking for success or for failure? Impossible to tell. If foo()
> returns a pointer, it almost certainly checks for success. If it
> returns bool, likewise. But if it returns an integer, it probably
> checks for failure.
>
> Getting a condition backwards is a common coding mistake. Consider
> patch review of
>
> if (condition) {
> obviously the error case
> }
>
> Patch review is more likely to catch a backward condition when the
> condition's sense is locally obvious.
>
> Conventions can help. Here's the one I like:
>
> * Check for a function's failure the same way everywhere.
>
> * When a function returns something "truthy" on success, and something
> "falsy" on failure, use
>
> if (!fun(...))
>
> Special cases:
>
> - bool true on success, false on failure
>
> - non-null pointer on success, null pointer on failure
>
> * When a function returns non-negative value on success, negative value
> on failure, use
>
> if (fun(...) < 0)
>
> * Avoid non-negative integer error values.
>
> * Avoid if (fun(...)), because it's locally ambiguous.
>
>> @Peter, Juan
>>
>> I'd like to hear your opinions.
>
> Yes, please.
I agree with Markus here for three reasons:
1 - He is my C - lawyer of reference O-)
2 - He has done a lot of work cleaning the error handling on this file,
that was completely ugly.
3 - I fully agree that code is more maintenable this way. I.e. if any
function changes to return positive values for non error, we get
better coverage.
Later, Juan.
