Closed Bug 415037 Opened 14 years ago Closed 14 years ago
"success" returned uninitialized from XPCVariant::Variant
Data To JS
Fired up my debug build post-FF3b3 crash landings and hit an uninitialized variable "success" referenced in the return statement of XPCVariant::VariantDataToJS On the stack was nsSAXXMLReader, apparently processing "http://newsrss.bbc.co.uk/rss/newsonline_world_edition/front_page/rss.xml" (redirected from http://fxfeeds.mozilla.org/rss20.xml). Specifically it's calling its HandleEndElement, which after a layer of js engine calls ends up in XPCVariant::VariantDataToJS with a T_INTERFACE_IS. In the "last ditch chance to prevent us from double-wrapping" chunk it IsWrappedJS and appears to successfully come up with a JSObject (though doesn't check the return value from GetJSObject--I guess a wrapper can't lose it's guts?). Since it _is_ an object it skips over the "if not an object" chunk that sets success, and thus returns with success uninitialized. Setting success=PR_TRUE; after getting the object seems to work, but are there other cases in the bit switch that don't return but wouldn't fall into the "if(xpctvar.type.TagPart() == TD_PSTRING_SIZE_IS" bit? If so might be safer to also initialize success up at the top.
(In reply to comment #0) > though doesn't check the > return value from GetJSObject--I guess a wrapper can't lose it's guts? It can, but then the QI just above will probably fail.
So what to do with this bug?
Let's just fix this.
Comment on attachment 301731 [details] [diff] [review] v1 Trivial fix for an unitialized variable.
Attachment #301731 - Flags: approval1.9?
This looks OK, but you have to do a lot of work to satisfy yourself that it's OK (check all the types that break out of the switch and verify they'll fall into the various code paths). Unless there's a huge perf bottleneck here it'd be safer to also initialize success to PR_FALSE up at the top, just a little future-proofing.
(In reply to comment #5) > This looks OK, but you have to do a lot of work to satisfy yourself that it's > OK (check all the types that break out of the switch and verify they'll fall > into the various code paths). I don't think it's that complicated. Once you're out of the switch, there's three possibilities: some strings, wrapped JS with a valid JS object, anything else. All three set success now.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Priority: -- → P3
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.