Closed Bug 617919 Opened 10 years ago Closed 9 years ago

ImplicitConvert should (void) its call to JS_IdToValue() to hint that it does not need to check the result

Categories

(Core :: XPConnect, enhancement)

enhancement
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

743 bytes, patch
timeless
: review+
Details | Diff | Splinter Review
1717 ImplicitConvert(JSContext* cx,
1718                 jsval val,
1719                 JSObject* targetType,
1720                 void* buffer,
1721                 bool isArgument,
1722                 bool* freePointer)
1723 {

1745   switch (targetCode) {
1966   case TYPE_struct: {
1967     if (!JSVAL_IS_PRIMITIVE(val) && !sourceData) {

1984       jsid id;

1986       while (1) {

1989         if (JSID_IS_VOID(id))
1990           break;
1991 
1992         js::AutoValueRooter fieldVal(cx);
1993         JS_IdToValue(cx, id, fieldVal.jsval_addr());
1994         if (!JSVAL_IS_STRING(fieldVal.jsval_value())) {
1995           JS_ReportError(cx, "property name is not a string")

Since you're relying on jsval_value() to be non string on failure, simply adding a (void) will make coverity happy.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #496576 - Flags: review?(jorendorff)
Use JS_ALWAYS_TRUE(...) instead. That compiles to ((void) (...)) in release builds, but JS_ASSERT(...) in debug builds.

r=me with that.
Comment on attachment 496576 [details] [diff] [review]
patch

Setting r+ bit, but not checkin-needed since this still needs a minor tweak.
Attachment #496576 - Flags: review?(jorendorff) → review+
Attached patch for checkinSplinter Review
Attachment #496576 - Attachment is obsolete: true
Attachment #498306 - Flags: review+
Keywords: checkin-needed
the code is gone
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.