netdev - Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <dcfa0fae-1321-1583-cb46-1665e1a4ea93@linaro.org>
Date:   Mon, 12 Apr 2021 02:45:38 -0500
From:   Alex Elder <elder@...aro.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     davem@...emloft.net, kuba@...nel.org, bjorn.andersson@...aro.org,
        evgreen@...omium.org, cpratapa@...eaurora.org,
        subashab@...eaurora.org, elder@...nel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an
 error

On 4/12/21 2:26 AM, Leon Romanovsky wrote:
> On Sun, Apr 11, 2021 at 08:42:15AM -0500, Alex Elder wrote:
>> On 4/11/21 8:28 AM, Leon Romanovsky wrote:
>>>> I think *not* checking an available return value is questionable
>>>> practice.  I'd really rather have a build option for a
>>>> "__need_not_check" tag and have "must_check" be the default.
>>> __need_not_check == void ???
>>
>> I'm not sure I understand your statement here, but...
> 
> We are talking about the same thing. My point was that __need_not_check
> is actually void. The API author was supposed to declare that by
> declaring that function doesn't return anything.

No, we are not.

Functions like strcpy() return a value, but that value is almost
never checked.  The returned value isn't an error, so there is
no real need to check that return value.  This is the kind of
thing I'm talking about that might be tagged __need_not_check.

A function that returns a value for no reason should be void,
I agree with that.

In the ipa_stop() case, the value *must* be returned because
it serves as an ->ndo_stop() function and has to adhere to
that function prototype.  The point of the current patch
was to simplify the code (defined privately in the current
source file), given knowledge that it never returns an error.

The compiler could ensure all calls to functions that return
a value actually check the return value.  And because I think
that's the best practice, I'd like to be able to run such a
check in my code.  But there are always exceptions, and that
would be the purpose of a __need_not_check tag.

I don't think this is worthy of any more discussion.

					-Alex