cppcheck finds some interesting stuff that is worth fixing. Generally the fixes are tiny so I'm going to create a single bug report for lots of fixes rather than dozens of tiny bugs reports.

cppcheck identified the extra ')' at the end of these macros. This code must not be compiled in any of our standard configurations.

cppcheck says: > b2g/app/nsBrowserApp.cpp:251: error: Uninitialized variable: gotCounters > browser/app/nsBrowserApp.cpp:637: error: Uninitialized variable: gotCounters It's a false positive because one of XP_WIN and XP_UNIX is always defined, but it doesn't hurt to make that fact clearer.

Comment on attachment 8551563 [details] [diff] [review] Fix syntax error in BT_WARNING and BT_LOGD Review of attachment 8551563 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Thanks for finding the nit.

cppcheck found this. Not important, but doesn't hurt to fix it.

I think these undefined variables don't matter because it's a static analysis input, but defining them does stop cppcheck from complaining.

Comment on attachment 8551568 [details] [diff] [review] Added #error cases for impossible platforms Review of attachment 8551568 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/app/nsBrowserApp.cpp @@ +223,5 @@ > #elif defined(XP_WIN) > IO_COUNTERS ioCounters; > gotCounters = GetProcessIoCounters(GetCurrentProcess(), &ioCounters); > +#else > + #error "unknown platform" Capital U to unknown. Add a comment that this is to make cppcheck happy, because a casual reader might end up wanting to remove the useless #else if he doesn't know where it comes from.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec1bdd395b2d https://hg.mozilla.org/integration/mozilla-inbound/rev/42df041f636c

https://hg.mozilla.org/mozilla-central/rev/ec1bdd395b2d https://hg.mozilla.org/mozilla-central/rev/42df041f636c

Comment on attachment 8551570 [details] [diff] [review] Fix a trivial leak Review of attachment 8551570 [details] [diff] [review]: ----------------------------------------------------------------- Cool!

Comment on attachment 8551571 [details] [diff] [review] Initialized some locals in TestNoArithmeticExprInArgument.cpp Review of attachment 8551571 [details] [diff] [review]: ----------------------------------------------------------------- Is there any way to instruct cppcheck to not look at the test files in this directory? These files are never built, only syntax-parsed: <https://dxr.mozilla.org/mozilla-central/source/build/clang-plugin/tests/Makefile.in#5> But I guess this is harmless (and useless. ;-)

> Is there any way to instruct cppcheck to not look at the test files in this > directory? Yes, the -i option can be used to ignore files and/or directories.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa7dbc827709 https://hg.mozilla.org/integration/mozilla-inbound/rev/34fe493c5806 https://hg.mozilla.org/integration/mozilla-inbound/rev/7da2cd7a1125

https://hg.mozilla.org/mozilla-central/rev/fa7dbc827709 https://hg.mozilla.org/mozilla-central/rev/34fe493c5806 https://hg.mozilla.org/mozilla-central/rev/7da2cd7a1125

cppcheck found this.

cppcheck found these.

Comment on attachment 8552837 [details] [diff] [review] Fix syntax error when NS_COORD_IS_FLOAT is defined Review of attachment 8552837 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! r=me

(In reply to Nicholas Nethercote [:njn] from comment #16) > Created attachment 8552848 [details] [diff] [review] > Fix syntax errors when DEBUG_LAYOUT is defined This is an empty patch. Forgot to qref?

cppcheck found this.

Comment on attachment 8552860 [details] [diff] [review] Fix syntax errors when DEBUG_LAYOUT is defined Stealing review, since I'm already here, and this is a trivial fix. >--- a/layout/xul/nsBoxFrame.cpp >@@ -1461,23 +1461,24 @@ nsBoxFrame::PaintXULDebugBackground(nsRe > r.y = r.y + r.height - debugBorder.bottom; > r.height = debugBorder.bottom; > drawTarget->FillRect(NSRectToRect(r, appUnitsPerDevPixel), color); > > // if we have dirty children or we are dirty > // place a green border around us. > if (NS_SUBTREE_DIRTY(this)) { > nsRect dirty(inner); >- ColorPattern green(ToDeviceColor(Color0.f, 1.f, 0.f, 1.f))); >+ ColorPattern green(ToDeviceColor(Color0.f, 1.f, 0.f, 1.f)); ^^^^^^^^ You're removing a close-paren here, but really this is missing an open-paren (after "Color" in "Color0.f"). r=me with that open-paren added (and the removed close-paren restored)

Comment on attachment 8552859 [details] [diff] [review] Fix syntax error when ERR_REPORTING_SYSLOG is defined Review of attachment 8552859 [details] [diff] [review]: ----------------------------------------------------------------- Sure, though we'd never define that.

https://hg.mozilla.org/mozilla-central/rev/9083ef23526a https://hg.mozilla.org/mozilla-central/rev/79010299a931 https://hg.mozilla.org/mozilla-central/rev/b64e7b3d671a

I've done as much here as I'm going to.