From: Boaz Harrosh <bharrosh@panasas.com>
To: Jan Beulich <jbeulich@novell.com>
Cc: "David S. Miller" <davem@davemloft.net>,
Ingo Molnar <mingo@elte.hu>,
Alexey Dobriyan <adobriyan@gmail.com>,
Ivo van Doorn <IvDoorn@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Theodore Tso <tytso@mit.edu>,
Rusty Russell <rusty@rustcorp.com.au>,
"John W. Linville" <linville@tuxdriver.com>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/5] debug: BUILD_BUG_ON: error on non-const expressions
Date: Mon, 01 Sep 2008 18:00:15 +0300 [thread overview]
Message-ID: <48BC037F.5010305@panasas.com> (raw)
In-Reply-To: <48BC1A10.76E4.0078.0@novell.com>
Jan Beulich wrote:
>>>> Boaz Harrosh <bharrosh@panasas.com> 01.09.08 16:21 >>>
>>> I have to admit that I'm surprise this compiles: You replace an expression
>>> with a statement, and hence you reduce the places where BUILD_BUG_ON()
>>> can validly be used.
>> it is only an expression because of the (void)() cast, which is what
>> I'm trying to avoid.
>
> No, sizeof() alone is an expression, too.
Using only sizeof() produces tons of "expression has no effect" warning
all over the place.
> Also, by using a statement you'll
> have more problems with fixing BUILD_BUG_ON_ZERO(), which must be
> an expression.
>
What is broken with my BUILD_BUG_ON_ZERO(). I tried all tests and
it works fine. Do you have a test with unwanted results?
(Actually it's the original one I have not touched it).
>>> Of course you could wrap the whole thing in ({}),
>> "do{}while(0)" is effectively an "{}" plus the added bonus
>> of demanding an ";" ;-)
>
> An expression likewise demands a terminating ; (or a continuation of the
> expression, i.e. by using an operator)
>
I was not criticizing your approach, I was commenting on:
"{}" vs. do{}while(0)
>>> Also, are you sure the compiler will eliminate the dead variable in all
>>> cases?
>>>
>>> Finally, using as common a variable as 'x' here seems dangerous, too:
>>> What if somewhere x is #define-d to something more complex than a
>>> simple identifier?
>> No it is scoped in a dead do{}while(0). What gets optimized out most
>> is the do nothing do{}while(0). The inside is just ignored.
>
> I don't think compilers in general and gcc in particular work this way
> (i.e. automatically throwing away everything included in a dead block).
>
I've tested and nothing is produced, even without any optimization.
But I'm not an expert.
> Jan
>
Boaz
next prev parent reply other threads:[~2008-09-01 15:01 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-01 13:00 [PATCHSET 0/5] BUILD_BUG_ON: error on none-const expressions Boaz Harrosh
2008-09-01 13:07 ` [PATCH] debug, x86: move BUILD_BUG_ON() ARRAY_SIZE and __FUNCTION__ Boaz Harrosh
2008-09-01 13:11 ` [PATCH 2/5] net/niu: Fix none-const BUILD_BUG_ON usage Boaz Harrosh
2008-09-01 13:13 ` [PATCH 3/5] virtio: " Boaz Harrosh
2008-09-02 15:53 ` [PATCH 3/5 ver2] virtio: Fix non-const " Boaz Harrosh
2008-09-01 13:21 ` [PATCH 4/5] rt2x00: Compiler warning unmasked by fix of BUILD_BUG_ON Boaz Harrosh
2008-09-01 13:27 ` Ivo Van Doorn
2008-09-01 13:27 ` Benny Halevy
2008-09-01 13:44 ` [PATCH 4/5 ver2] " Boaz Harrosh
2008-09-01 14:01 ` Ivo Van Doorn
2008-09-01 15:17 ` Boaz Harrosh
2008-09-01 16:34 ` Ivo van Doorn
2008-09-02 14:20 ` Boaz Harrosh
2008-09-02 14:27 ` Ivo van Doorn
2008-09-02 15:55 ` Boaz Harrosh
2008-09-01 13:28 ` [PATCH 5/5] debug: BUILD_BUG_ON: error on non-const expressions Boaz Harrosh
2008-09-01 13:55 ` Jan Beulich
2008-09-01 14:21 ` Boaz Harrosh
2008-09-01 14:36 ` Jan Beulich
2008-09-01 15:00 ` Boaz Harrosh [this message]
2008-09-01 15:29 ` Jan Beulich
2008-09-01 16:41 ` Boaz Harrosh
2008-09-02 7:47 ` Jan Beulich
2008-09-02 15:19 ` Boaz Harrosh
2008-09-02 15:57 ` [PATCH 5/5 ver2] " Boaz Harrosh
2008-09-02 16:06 ` Boaz Harrosh
2008-09-02 16:11 ` Jan Beulich
2008-09-03 8:57 ` Boaz Harrosh
2008-09-03 10:19 ` Jan Beulich
2008-09-03 10:52 ` Boaz Harrosh
2008-10-02 5:35 ` Rusty Russell
2008-10-05 9:34 ` Boaz Harrosh
2008-09-02 16:07 ` [PATCH 5/5 ver3] " Boaz Harrosh
2008-09-06 16:01 ` [PATCHSET 0/5] BUILD_BUG_ON: error on none-const expressions Ingo Molnar
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=48BC037F.5010305@panasas.com \
--to=bharrosh@panasas.com \
--cc=IvDoorn@gmail.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=jbeulich@novell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=mingo@elte.hu \
--cc=rusty@rustcorp.com.au \
--cc=torvalds@linux-foundation.org \
--cc=tytso@mit.edu \
/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.