funobj is uselessly set in WrapFunction

RESOLVED FIXED in mozilla1.9.3a5

Status

()

Core
XPConnect
--
minor
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({coverity})

Trunk
mozilla1.9.3a5
coverity
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking2.0 final+)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
412 WrapFunction(JSContext *cx, JSObject *scope, JSObject *funobj, jsval *rval)

419   if (native == XPC_COW_FunctionWrapper) {

426     funobj = JSVAL_TO_OBJECT(funobjVal);
427   }
428 
429   JSFunction *funWrapper =
430     JS_NewFunction(cx, XPC_COW_FunctionWrapper,
431                    JS_GetFunctionArity(wrappedFun), 0,
432                    scope,
433                    JS_GetFunctionName(wrappedFun));
434   if (!funWrapper) {
435     return JS_FALSE;
436   }
437 
438   JSObject *funWrapperObj = JS_GetFunctionObject(funWrapper);
439   *rval = OBJECT_TO_JSVAL(funWrapperObj);
440 
441   return JS_SetReservedSlot(cx, funWrapperObj,
442                             XPCWrapper::eWrappedFunctionSlot,
443                             funobjVal);
444 }
Should we be using that (but fail to) or is this just a set that shouldn't be there?

Requesting blocking in case it's the former...
blocking2.0: --- → ?
(Assignee)

Comment 2

8 years ago
Created attachment 439863 [details] [diff] [review]
proposal

so, jsvals are aliases to jsobjects. i'm hoping it's just someone trying to keep the function argument in sync with the local jsobject. I'd rather we have a DO_NOT_USE(function_argument) statement at the point where it's stale and rely on static analysis to complain if it's used. The current approach risks compilers not optimizing away the useless bookkeeping we have. -- Assuming I'm right that it's useless (instead of errantly unused).
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #439863 - Flags: review?(mrbkap)
Comment on attachment 439863 [details] [diff] [review]
proposal

I think that I might have wanted to re-load wrappedFun out of funobj (as well as just keep it updated), but since we only use wrappedFun for its name and arity, this is fine.
Attachment #439863 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/mozilla-central/rev/996419229468
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5

Updated

8 years ago
blocking2.0: ? → final+
You need to log in before you can comment on or make changes to this bug.