Closed Bug 1277734 Opened 3 years ago Closed 2 years ago

Null test after dereference in CompartmentChecker::check

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox49 --- affected
firefox58 --- fixed

People

(Reporter: njn, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, triage-deferred, Whiteboard: CID 1260199)

Attachments

(1 file)

Coverity found this:

>   void check(JSCompartment* c) {
>       if (c && !compartment->runtimeFromAnyThread()->isAtomsCompartment(c)) {
>           if (!compartment)
>               compartment = c;
>           else if (c != compartment)
>               fail(compartment, c);
>       }
>   }

No point checking is |compartment| is null after we've dereferenced it.
Assignee: nobody → jorendorff
Keywords: triage-deferred
Priority: -- → P3
Comment on attachment 8914797 [details] [diff] [review]
Null test after dereference in CompartmentChecker::check

Review of attachment 8914797 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jscntxtinlines.h
@@ +58,1 @@
>              if (!compartment)

The fact that we're not crashing suggests that we could remove this branch since |compartment| is never null here.
Attachment #8914797 - Flags: review?(wmccloskey) → review+
I had a reason for doing it this way, but on closer inspection it turned out to be a bad reason. So I deleted the extra branch.
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e7ba9d2bdc0
Null test after dereference in CompartmentChecker::check. r=billm
https://hg.mozilla.org/mozilla-central/rev/5e7ba9d2bdc0
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.