useless check of !data.index in nsContentUtils::ReparentClonedObjectToScope

RESOLVED FIXED

Status

()

Core
DOM
--
trivial
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({coverity})

Trunk
coverity
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment, 1 obsolete attachment)

832 bytes, patch
timeless
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
5822 nsContentUtils::ReparentClonedObjectToScope(JSContext* cx,
5833   while (!objectData.IsEmpty()) {
5836     if (!data.ids && !data.index) {
5874     }
5878     if (data.index == data.ids->length) {
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#5871 Sets data.ids or exits if it fails. INVALID.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → INVALID
(Assignee)

Comment 2

8 years ago
you're assuming data.index == 0, what if data.index isn't 0?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
data.index is initialized to 0 and it's only ever set to a nonzero value if data.ids is nonzero. Since it's impossible to have a null data.ids without failing I don't see any bug here. Please correct me or close this bug as appropriate.
(Assignee)

Updated

8 years ago
Assignee: nobody → timeless
Severity: critical → trivial
Status: REOPENED → ASSIGNED
Keywords: crash
Summary: crash [@ nsContentUtils::ReparentClonedObjectToScope] if !data.ids → useless check of !data.index in nsContentUtils::ReparentClonedObjectToScope
(Assignee)

Comment 4

8 years ago
Created attachment 443371 [details] [diff] [review]
patch
Attachment #443371 - Flags: review?(bent.mozilla)
Comment on attachment 443371 [details] [diff] [review]
patch

>-    if (!data.ids && !data.index) {
>+    if (!data.ids) {

Can you add NS_ASSERTION(!data.index, "Shouldn't have index here") right here? r=me with that.
Attachment #443371 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 6

8 years ago
Created attachment 443459 [details] [diff] [review]
patch
Attachment #443371 - Attachment is obsolete: true
Attachment #443459 - Flags: review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7c01659eb8d9
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [build_warning]
You need to log in before you can comment on or make changes to this bug.