422484 - Add parens around macro if conditions

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[David Mandelin [:dmandelin]]

Attachment: Patch to add parens to macro if conditions

There are 2 places in SVG where if conditions are "missing" parens, like

  if NS_FAILED(rv) { ...

This compiles fine, because of the definition of the macro, but it looks funny. More importantly, it prevents NS_FAILED from being an inline function, which makes static analysis harder. 

So, can we add them, as in the attached patch? mozilla-central is where I really want this, that's the main target for static analysis, but it might be good to have it in CVS trunk as well.

Comment 1

[Benjamin Smedberg]

Comment on attachment 308926 [details] [diff] [review]
Patch to add parens to macro if conditions

This is trivially correct. I've already fixed two of these! Reviewers should be catching these :-(

Comment 2

[Daniel Holbert [:dholbert]]

FWIW, here's 2 more of this same type of bug, in Camino code:

http://mxr.mozilla.org/seamonkey/source/camino/src/formfill/wallet.mm#840
 840   if NS_FAILED(rv) {
 854   if NS_FAILED(rv) {

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: patch for same issue in camino code

Comment 4

[Robert Longson [:longsonr]]

The nsSVGGlyphFrame code will disappear anyway as a side effect of bug 392233

Comment 5

[Damon Sicore (:damons)]

Comment on attachment 308926 [details] [diff] [review]
Patch to add parens to macro if conditions

a1.9+=damons

Comment 6

[Josh Aas]

Comment on attachment 308939 [details] [diff] [review]
patch for same issue in camino code

wow, nice

Comment 7

[Daniel Holbert [:dholbert]]

Both patches checked in.

Checking in svg/base/src/nsSVGGlyphFrame.cpp;
/cvsroot/mozilla/layout/svg/base/src/nsSVGGlyphFrame.cpp,v  <--  nsSVGGlyphFrame.cpp
new revision: 1.119; previous revision: 1.118
done
Checking in xul/base/src/nsXULPopupManager.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsXULPopupManager.cpp,v  <--  nsXULPopupManager.cpp
new revision: 1.54; previous revision: 1.53
done


Checking in wallet.mm;
/cvsroot/mozilla/camino/src/formfill/wallet.mm,v  <--  wallet.mm
new revision: 1.11; previous revision: 1.10
done