NPObjectMember_Convert is the only function that doesn't null check JS_GetInstancePrivate

RESOLVED FIXED

Status

()

Core
Plug-ins
--
critical
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({coverity, crash})

Trunk
coverity, crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
you assert but don't check here:
2079 NPObjectMember_Convert(JSContext *cx, JSObject *obj, JSType type, jsval *vp)
2082     (NPObjectMemberPrivate *)::JS_GetInstancePrivate(cx, obj,
2083                                                      &sNPObjectMemberClass,
2084                                                      nsnull);
2085   NS_ASSERTION(memberPrivate, "no Ambiguous Member Private data!");
2093     *vp = memberPrivate->fieldValue;

whereas you null check:
2117 NPObjectMember_Call(JSContext *cx, JSObject *obj,
2124 (NPObjectMemberPrivate *)::JS_GetInstancePrivate(cx, memobj,
2125 &sNPObjectMemberClass,
2126 argv);
2127 if (!memberPrivate || !memberPrivate->npobjWrapper)
2128     return JS_FALSE;

and you null check:
2198 NPObjectMember_Mark(JSContext *cx, JSObject *obj, void *arg)
2201 (NPObjectMemberPrivate *)::JS_GetInstancePrivate(cx, obj,
2202 &sNPObjectMemberClass,
2203 nsnull);
2204 if (!memberPrivate)
2205     return 0;

if you really don't think this class can ever have nulls, then please improve our speed by dropping the null checks, but if there's any chance, please change the assert to a null check.
(Assignee)

Comment 1

9 years ago
Created attachment 390185 [details] [diff] [review]
proposal
Assignee: nobody → timeless
Attachment #390185 - Flags: review?(joshmoz)

Updated

9 years ago
Attachment #390185 - Flags: review?(joshmoz) → review?(jst)

Updated

9 years ago
Attachment #390185 - Flags: superreview+
Attachment #390185 - Flags: review?(jst)
Attachment #390185 - Flags: review+
(Assignee)

Comment 2

9 years ago
http://hg.mozilla.org/mozilla-central/rev/1d550f0cf16c
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.