From: Alexey Dobriyan <adobriyan@sw.ru>
To: akpm@osdl.org, torvalds@osdl.org
Cc: linux-kernel@vger.kernel.org, herbert@gondor.apana.org.au
Subject: WARN_ON() which sometimes sucks
Date: Tue, 31 Jul 2007 19:55:27 +0400 [thread overview]
Message-ID: <20070731155527.GB7253@localhost.sw.ru> (raw)
It started when I tried to write
WARN_ON(m->seq_ops_allocated);
in today's "[PATCH] single_open/seq_release leak diagnostics¹.
Suprisingly compiler told me piss off with:
CC fs/seq_file.o
fs/seq_file.c: In function 'seq_release':
fs/seq_file.c:285: error: 'typeof' applied to a bit-field
Well, duh! Earlier versions of WARN_ON allowed that until commit
684f978347deb42d180373ac4c427f82ef963171² which added typeof().
OK, nobody noticed that WARN_ON(bitfield) stopped working. But I
question the rationale of that commit:
Letting WARN_ON/WARN_ON_ONCE return the condition means that you could
do
if (WARN_ON(blah)) {
handle_impossible_case
}
Rather than
if (unlikely(blah)) {
WARN_ON(1)
handle_impossible_case
}
I think that second case is more clear and immediately understandable.
If folks agree, I'll send revert so that WARN_ON(var), WARN_ON(ptr),
WARN_ON(bitfield) and WARN_ON(condition) work as expected.
¹ http://marc.info/?l=linux-kernel&m=118588389612083&w=2
²
tree db9025d8c6b267565c7110e09b16193957186a48
parent 6299a2dec89d22940e36832f15c0addfb77e6910
author Herbert Xu <herbert@gondor.apana.org.au> 1159520346 -0700
committer Linus Torvalds <torvalds@g5.osdl.org> 1159546686 -0700
[PATCH] Let WARN_ON/WARN_ON_ONCE return the condition
Letting WARN_ON/WARN_ON_ONCE return the condition means that you could do
if (WARN_ON(blah)) {
handle_impossible_case
}
Rather than
if (unlikely(blah)) {
WARN_ON(1)
handle_impossible_case
}
I checked all the newly added WARN_ON_ONCE users and none of them test the
return status so we can still change it.
[akpm@osdl.org: warning fix]
[akpm@osdl.org: build fix]
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
next reply other threads:[~2007-07-31 15:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-31 15:55 Alexey Dobriyan [this message]
2007-07-31 16:02 ` WARN_ON() which sometimes sucks Al Viro
2007-08-01 3:53 ` Paul Mackerras
2007-08-01 4:05 ` Linus Torvalds
2007-08-01 4:20 ` Paul Mackerras
2007-08-01 15:17 ` Joe Korty
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070731155527.GB7253@localhost.sw.ru \
--to=adobriyan@sw.ru \
--cc=akpm@osdl.org \
--cc=herbert@gondor.apana.org.au \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@osdl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.