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: <2f439541-154f-1f17-180e-db0d8f95e010@gmail.com>
Date:   Wed, 12 Sep 2018 18:04:30 +0300
From:   Igor Stoppa <igor.stoppa@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     igor.stoppa@...wei.com, huangdaode@...ilicon.com,
        yisen.zhuang@...wei.com, salil.mehta@...wei.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ethernet: hnae: drop adhoc assert() macros

On 12/09/18 09:07, David Miller wrote:
> From: Igor Stoppa <igor.stoppa@...il.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@...wei.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ