Closed Bug 604636 Opened 9 years ago Closed 9 years ago

nsJSCID::HasInstance bails too strictly: enforces both other_wrapper are obj2 non null instead of one is

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

718 bytes, patch
mrbkap
: review+
benjamin
: approval2.0+
Details | Diff | Splinter Review
883 nsJSCID::HasInstance(nsIXPConnectWrappedNative *wrapper,
884                      JSContext * cx, JSObject * obj,
885                      const jsval &val, PRBool *bp, PRBool *_retval)

899         XPCWrappedNative* other_wrapper =
900            XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj, nsnull, &obj2);
901 
902         if(!other_wrapper || !obj2)
903             return NS_OK;

useless null check of other_wrapper:
905         nsIClassInfo* ci = other_wrapper ?
906                            other_wrapper->GetClassInfo() :
907                            GetSlimWrapperProto(obj2)->GetClassInfo();
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #483787 - Flags: review?(mrbkap)
Comment on attachment 483787 [details] [diff] [review]
patch

It looks like the if statement is wrong. If there's no wrapped native, but we did find an obj2 (slim wrapper), then we can still check instanceof here.
Attachment #483787 - Flags: review?(mrbkap) → review-
Summary: nsJSCID::HasInstance has stale code that tries to null check other_wrapper → nsJSCID::HasInstance bails too strictly: enforces both other_wrapper are obj2 non null instead of one is
Attached patch per mrbkapSplinter Review
Attachment #483787 - Attachment is obsolete: true
Attachment #483808 - Flags: review?(mrbkap)
Attachment #483808 - Flags: approval2.0?
Attachment #483808 - Flags: review?(mrbkap) → review+
Attachment #483808 - Flags: approval2.0? → approval2.0+
Pushed http://hg.mozilla.org/mozilla-central/rev/30b83f45346c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.