"warning: invoking macro NS_ENSURE_SUCCESS argument 2: empty macro arguments are undefined in ISO C90 and ISO C++98" in DOM worker code

RESOLVED FIXED in mozilla1.9.3a2

Status

()

defect
--
minor
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Waldo, Assigned: wesongathedeveloper)

Tracking

Trunk
mozilla1.9.3a2
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(2 attachments)

DOM worker code seems to use this:

  NS_ENSURE_SUCCESS(rv,)

inside some constructors as a way to error-check, assuming that providing an empty parameter expands to nothing (or at best, whitespace).  This apparently isn't language-kosher, so I get these warnings (and many others in other files) when compiling:

/home/jwalden/moz/2/dom/src/threads/nsDOMThreadService.cpp:599:26: warning: invoking macro NS_ENSURE_SUCCESS argument 2: empty macro arguments are undefined in ISO C90 and ISO C++98
/home/jwalden/moz/2/dom/src/threads/nsDOMThreadService.cpp:599:26: warning: invoking macro NS_ENSURE_SUCCESS argument 2: empty macro arguments are undefined in ISO C90 and ISO C++98

It looks like these should just be expanded to avoid using the macro incorrectly:

  if (NS_FAILED(rv))
    return;
[jwalden@the-great-waldo-search js-tm]$ grep -r 'NS_ENSURE_SUCCESS\(.*,[[:space:]]*\);' .
./dom/src/threads/nsDOMWorkerTimeout.cpp:  NS_ENSURE_SUCCESS(*aRv,);
./dom/src/threads/nsDOMWorkerTimeout.cpp:  NS_ENSURE_SUCCESS(*aRv,);
./dom/src/threads/nsDOMWorkerTimeout.cpp:  NS_ENSURE_SUCCESS(*aRv,);
./dom/src/threads/nsDOMThreadService.cpp:    NS_ENSURE_SUCCESS(rv,);
./dom/src/threads/nsDOMThreadService.cpp:  NS_ENSURE_SUCCESS(rv,);
./dom/src/threads/nsDOMThreadService.cpp:  NS_ENSURE_SUCCESS(rv,);
./accessible/src/base/nsDocAccessible.cpp:    NS_ENSURE_SUCCESS(rv,);
./content/smil/nsSMILAnimationFunction.cpp:    NS_ENSURE_SUCCESS(InterpolateResult(values, result, aResult),);
./content/smil/nsSMILAnimationFunction.cpp:    NS_ENSURE_SUCCESS(AccumulateResult(values, result),);
./xpcom/tests/TestTimers.cpp:    NS_ENSURE_SUCCESS(rv,);
./xpcom/tests/TestProxies.cpp:    NS_ENSURE_SUCCESS(rv,);
./xpcom/tests/TestProxies.cpp:    NS_ENSURE_SUCCESS(rv,);

Unless my pattern is messed up (entirely possible, grep patterns, shell syntax, and I don't get along well), that's a complete list.
Component: DOM: Mozilla Extensions → General
QA Contact: general → general
Whiteboard: [good first bug]
Posted patch PatchSplinter Review
Assignee: nobody → wesongathedeveloper
Status: NEW → ASSIGNED
Attachment #408304 - Flags: review?(jst)
Attachment #408304 - Flags: review?(jst) → review+
Applies cleanly
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/d80b2bb478dc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Why not use:
  NS_ENSURE_SUCCESS(rv, /**/);
instead, so we don't lose the NS_WARN_IF_FALSE?
Does whitespace plus comment equal non-empty macro argument?  (For some reason my compiler is no longer warning on the stupid testcase I wrote to test this, which either means I'm not writing it correctly or [I suspect more likely] a compiler update took away the warning.)
You need to log in before you can comment on or make changes to this bug.