723530 - double error reporting in ctype JS api implementation

Status: RESOLVED FIXED

(Core :: js-ctypes, defect)

Description

[Igor Bukanov]

In many places ctypes implementation contains the following code:

  JSObject* obj = JS_THIS_OBJECT(cx, vp);
  if (!obj || !(CType::IsCType(obj) || CType::IsCTypeProto(obj))) {
    JS_ReportError(cx, "not a CType");
    return JS_FALSE;
  }

That is, the error is reported when JS_THIS_OBJECT(cx, vp) returns false. However, JS_THIS_OBJECT(cx, vp) already reports an error when it fails leading to bogus second error report and stopping of OOM propagation that triggered JS_THIS_OBJECT failure.

Comment 1

[Josh Matthews [:jdm]]

I assume the correct fix is to move the null check out of the condition and into an early return?

Comment 2

[Igor Bukanov]

(In reply to Josh Matthews [:jdm] from comment #1)
> I assume the correct fix is to move the null check out of the condition and
> into an early return?

Yes

Comment 3

[Mark Capella [:capella]]

Attachment: Patch (v1)

Have a look at the patch, I think it's what you're after.

It builds clean and mochitest-plain passes locally on my WIN7 machine.

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 619146 [details] [diff] [review]
Patch (v1)

Yep, that looks right to me. Onwards, brave reviewers!

Comment 5

[Josh Matthews [:jdm]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/100e249720c1

Comment 6

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/100e249720c1