netdev - Re: [PATCH net-next] cirrus: cs89x0: fix the return value handle and remove redundant dev_warn() for platform_get

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]

Date: Wed, 02 Aug 2023 22:12:34 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Alex Elder" <elder@...aro.org>, "Simon Horman" <horms@...nel.org>,
 "Ruan Jinjie" <ruanjinjie@...wei.com>
Cc: "David S . Miller" <davem@...emloft.net>,
 "Eric Dumazet" <edumazet@...gle.com>, "Jakub Kicinski" <kuba@...nel.org>,
 "Paolo Abeni" <pabeni@...hat.com>, "Wei Fang" <wei.fang@....com>,
 "Rob Herring" <robh@...nel.org>, bhupesh.sharma@...aro.org,
 Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] cirrus: cs89x0: fix the return value handle and remove
 redundant dev_warn() for platform_get_irq()

On Wed, Aug 2, 2023, at 15:33, Alex Elder wrote:
> On 8/2/23 3:32 AM, Simon Horman wrote:
>> On Tue, Aug 01, 2023 at 09:31:21PM +0800, Ruan Jinjie wrote:
>>> There is no possible for platform_get_irq() to return 0
>>> and the return value of platform_get_irq() is more sensible
>>> to show the error reason.
>>>
>>> And there is no need to call the dev_warn() function directly to print
>>> a custom message when handling an error from platform_get_irq() function as
>>> it is going to display an appropriate error message in case of a failure.
>>>
>>> Signed-off-by: Ruan Jinjie <ruanjinjie@...wei.com>
>
> First, I agree that the dev_warn() is unnecessary.
>
> On the "<" versus "<=" issue is something I've commented on before.
>
> It's true that 0 is not (or should not be) a valid IRQ number.  But
> at one time a several years back I couldn't convince myself that it
> 100% could not happen.  I no longer remember the details, and it
> might not have even been in this particular case (i.e., return
> from platform_get_irq()).
>
> I do see that a85a6c86c25be ("driver core: platform: Clarify that
> IRQ 0 is invalid)" got added in 2020, and it added a WARN_ON()
> in platform_get_irq_optional() before returning the IRQ number if
> it's zero.  So in this case, if it *did* happen to return 0,
> you'd at least get a warning.

Some of the older arm32 platforms used to start IRQ numbers at 0
instead of 1, but those should all have been converted by now,
and it's unlikely that the first interrupt would be the network
controller on any of those that did.

      Arnd