Closed Bug 782677 Opened 8 years ago Closed 8 years ago

CHECK_VALID_NODEINFO needs to be checked

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: mccr8, Assigned: mccr8)

Details

Attachments

(1 file, 1 obsolete file)

The macro CHECK_VALID_NODEINFO has a few problems I can see.

- The first thing I noticed is that after the conversion to nullptr, we get the following warning message in GCC:
  nsNodeInfoManager.cpp:259: warning: NULL used in arithmetic
This is because the macro does (_extraName != nullptr) when it should do !!_extraName.
- Looking at the macro, I noticed that the argument _namespaceID is unused, presumably because aNamespaceID later in the macro should be _namespaceID. Fortunately it doesn't really matter because every _namespaceID argument is aNamespaceID.
- This should probably be converted to an inline function, as that would avoid the previous problem.
Attached patch de-macro and fix (obsolete) — Splinter Review
Unfortunately, this requires including a few more headers, but maybe that's not a big deal.  I could also move the definition to the cpp and just lose the inlining, or just leave it as a sketchy macro.
Attached patch de-macro and fixSplinter Review
I forgot to fix the (foo != nullptr) thing, but apparently de-macroizing it shuts up the warning.  Awesome.  Fixed it now anyways.
Attachment #651806 - Attachment is obsolete: true
Comment on attachment 651809 [details] [diff] [review]
de-macro and fix

I can fix this another way if you prefer.
Attachment #651809 - Flags: review?(bugs)
Attachment #651809 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/75f5849d6495
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.