lkml.org 
[lkml]   [2018]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] ethernet: hnae: drop adhoc assert() macros
From
Date
On 12/09/18 09:07, David Miller wrote:
> From: Igor Stoppa <igor.stoppa@gmail.com>
> Date: Sat, 8 Sep 2018 18:01:42 +0300
>
>> Replace assert() with a less misleading test_condition() using WARN()
>> Drop one check which had bitrotted and didn't compile anymore.
>>
>> Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
>
> I'm still kind of not happy about this.
>
> Make the driver use kernel interfaces like WARN_ON_ONCE()
> etc. directly instead of defining alias CPP macros private to the
> driver.
>
> If it needs to be conditional upon DEBUG, we have pr_debug() and
> the likes as well.

WARN_ON_ONCE() will display the error message only once, but from its
implementation it looks like it will evaluate the condition anyways,
every time, unless the compiler can do some magic and somehow know that
the condition was true already onece (/include/asm-generic/bug.h:146)

pr_debug() otoh, will print or not print, depending on
CONFIG_DYNAMIC_DEBUG being set or not (/include/linux/printk.h:399)

but the logic we are trying to clean up here seems a bit different:

if (in_debug_mode && error_condition_is_verified)
generate_warning()

Which, I guess, is meant to completely remove *any* test outside of
debug mode.

That is why I used the test_condition() macro, instead of using directly
WARN_ON_ONCE(). Admittedly, it probably would have been better to
depend on CONFIG_DYNAMIC_DEBUG.

Neither using WARN_ON_ONCE(), nor pr_debug() seem to replicate the
initial behavior of the code that is currently in the tree, with assert().

How would you suggest that I keep the same behavior, without the helper
macro? I am probably missing it, but I do not see some facility that
would support the double condition I described above.

--
thanks, igor

\
 
 \ /
  Last update: 2018-09-12 17:05    [W:0.035 / U:4.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site