User Agent: Mozilla/5.0 (Ubuntu; X11; Linux i686; rv:8.0) Gecko/20100101 Firefox/8.0 Build ID: 20111115183158 Steps to reproduce: The following malformed conditional was found using Coccinelle (http://coccinelle.lip6.fr) in the file js/src/shell/js.cpp in the function ConvertArgs. The ; after the if test means that ReportException is always called. if (!tmpstr || !func.encode(cx, tmpstr)); ReportException(cx);
Nice catch. Some of the error-handling code in that method looks dodgy, but not enough to be sure that it's not that way intentionally so, so I'm not sure the obvious fix -- removing the ; -- is enough to be correct here.
Some compilers give a warning here -- this is a good one, worth error'ing on. SpiderMonkey house style uses continue; and not just ; to write empty loop bodies. /be
Brendan, that's not even a loop -- just an if statement. I'm *really* not sure why compilers aren't warning about that.
GCC has -Wempty-body, which is enabled by -Wextra but not -Wall.
(In reply to Jeff Walden (remove +bmo to email) from comment #3) > Brendan, that's not even a loop -- just an if statement. Yes, I know -- it ought to be always an error. > I'm *really* not sure why compilers aren't warning about that. Walso, should comment 4 be spun out as a new owned bug? Or fixed here, whatever is best. /be
I'm going to add -Wempty-body in bug 711895.
There are about 50 empty-bodied ifs in ipc/chromium/src/base/third_party/purify/pure_api.c. These all look intentional.
(In reply to Julia Lawall from comment #7) > There are about 50 empty-bodied ifs in > ipc/chromium/src/base/third_party/purify/pure_api.c. > These all look intentional. Yes, but that file is only compiled on Windows so if you turn on -Wempty-body for GCC it doesn't matter. I only get two extra warnings when I turn it on for Linux64 debug builds, both in gfx/ycbcr/yuv_convert.cpp.
Landed with visual review from dvander: http://hg.mozilla.org/integration/mozilla-inbound/rev/5c40bfc44022
(In reply to Nicholas Nethercote [:njn] from comment #8) > (In reply to Julia Lawall from comment #7) > > There are about 50 empty-bodied ifs in > > ipc/chromium/src/base/third_party/purify/pure_api.c. > > These all look intentional. > > Yes, but that file is only compiled on Windows so if you turn on > -Wempty-body for GCC it doesn't matter. I only get two extra warnings when > I turn it on for Linux64 debug builds, both in gfx/ycbcr/yuv_convert.cpp. In case this comes up in the future, you can temporarily disable warnings in GCC by wrapping affected code with #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wempty-body" #pragma GCC diagnostic pop