linux-kernel - Re: [PATCH] Fix WARN_ON / WARN_ON

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <452589C3.8000705@goop.org>
Date:	Thu, 05 Oct 2006 15:40:03 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Herbert Xu <herbert@...dor.apana.org.au>
CC:	Andrew Morton <akpm@...l.org>,
	"Ananiev, Leonid I" <leonid.i.ananiev@...el.com>,
	tim.c.chen@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix WARN_ON / WARN_ON_ONCE regression

Herbert Xu wrote:
> The original reason for the return value is so you can do
>
> if (WARN_ON(impossible_condition)) {
> 	attempt_to_continue;
> }
>
> instead of 
>
> if (unlikely(impossible_condition)) {
> 	WARN_ON(1);
> 	attempt_to_continue;
> }
>   

(Hm, WARN_ON(1) is pretty ugly; we should probably have a WARN() as well.)

Why is the second one any better than the first?  It's a line less code, 
but that doesn't seem like a big deal.  It's not like passing the actual 
condition into WARN_ON is useful, because it doesn't try to print it 
out. And "if (WARN_ON_ONCE(cond)) ..." is arguably more useful (since it 
encapsulates the printing once logic), but also very unclear (does it 
evaluate true once or every time?).

There are certainly lots of places in the kernel which could use 
if(WARN_ON(...)), but I haven't found any places which actually do.  I 
just don't see what benefit you would gain in converting things to using 
if(WARN_ON(...)) anyway.

> Oh and yes the unlikely does make a difference in a statement
> expression.
>   

I was thinking something like

	unlikely(({
		...
	}))

is a bit more obvious in terms of imagining how it would get expanded 
and evaluated.

    J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/