Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH net-next 4/7] net: ipa: ipa_stop() does not return an
 error
To:     Leon Romanovsky <leon@kernel.org>
Cc:     davem@davemloft.net, kuba@kernel.org, bjorn.andersson@linaro.org,
        evgreen@chromium.org, cpratapa@codeaurora.org,
        subashab@codeaurora.org, elder@kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20210409180722.1176868-1-elder@linaro.org>
 <20210409180722.1176868-5-elder@linaro.org> <YHKYWCkPl5pucFZo@unreal>
 <1f5c3d2c-f22a-ef5e-f282-fb2dec4479f3@linaro.org> <YHL5fwkYyHvQG2Z4@unreal>
 <6e0c08a0-aebd-83b2-26b5-98f7d46d6b2b@linaro.org> <YHP2IKZ7pB+l4a6O@unreal>
From:   Alex Elder <elder@linaro.org>
Message-ID: <dcfa0fae-1321-1583-cb46-1665e1a4ea93@linaro.org>
Date:   Mon, 12 Apr 2021 02:45:38 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <YHP2IKZ7pB+l4a6O@unreal>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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