Re: [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION

From: Leon Romanovsky
Date: Tue Jul 27 2021 - 07:16:48 EST


On Mon, Jul 26, 2021 at 12:40:06PM -0500, Alex Elder wrote:
> A few months ago I proposed cleaning up some code that validates
> certain things conditionally, arguing that doing so once is enough,
> thus doing so always should not be necessary.
> https://lore.kernel.org/netdev/20210320141729.1956732-1-elder@xxxxxxxxxx/
> Leon Romanovsky felt strongly that this was a mistake, and in the
> end I agreed to change my plans.

<...>

> The second patch fixes a bug that wasn't normally exposed because of
> the conditional compilation (a reason Leon was right about this).

Thanks Alex,

If you want another anti pattern that is very popular in netdev, the following pattern is
wrong by definition :):
if (WARN_ON(...))
return ...

The WARN_*() macros are intended catch impossible flows, something that
shouldn't exist. The idea that printed stack to dmesg and return to the
caller will fix the situation is a very naive one. That stack already
says that something very wrong in the system.

If such flow can be valid use "if(...) return ..", if not use plain
WARN_ON(...).

Thanks