Coverity: Stop those monkeyshines right now!

Giving someone Coverity is like giving them a monkeysuit, it doesn't make them a real soldier, and certainly not a good soldier.

diff -r e60330fab5fe js/src/jsdbgapi.cpp
--- a/js/src/jsdbgapi.cpp	Sun Jun 07 09:32:19 2009 +0300
+++ b/js/src/jsdbgapi.cpp	Mon Jun 22 13:50:36 2009 +0300
@@ -242,17 +242,16 @@ JS_ClearScriptTraps(JSContext *cx, JSScr
     DBG_LOCK(rt);
     for (trap = (JSTrap *)rt->trapList.next;
          &trap->links != &rt->trapList;
          trap = next) {
         next = (JSTrap *)trap->links.next;
         if (trap->script == script) {
             sample = rt->debuggerMutations;
             DestroyTrapAndUnlock(cx, trap);
-            DBG_LOCK(rt);
             if (rt->debuggerMutations != sample + 1)
                 next = (JSTrap *)rt->trapList.next;
         }
     }
     DBG_UNLOCK(rt);
 }
 
 JS_PUBLIC_API(void)
@@ -265,17 +264,16 @@ JS_ClearAllTraps(JSContext *cx)
     rt = cx->runtime;
     DBG_LOCK(rt);
     for (trap = (JSTrap *)rt->trapList.next;
          &trap->links != &rt->trapList;
          trap = next) {
         next = (JSTrap *)trap->links.next;
         sample = rt->debuggerMutations;
         DestroyTrapAndUnlock(cx, trap);
-        DBG_LOCK(rt);
         if (rt->debuggerMutations != sample + 1)
             next = (JSTrap *)rt->trapList.next;
     }
     DBG_UNLOCK(rt);
 }
 
 JS_PUBLIC_API(JSTrapStatus)
 JS_HandleTrap(JSContext *cx, JSScript *script, jsbytecode *pc, jsval *rval)
@@ -458,17 +456,16 @@ js_SweepWatchPoints(JSContext *cx)
          &wp->links != &rt->watchPointList;
          wp = next) {
         next = (JSWatchPoint *)wp->links.next;
         if (js_IsAboutToBeFinalized(cx, wp->object)) {
             sample = rt->debuggerMutations;
 
             * Ignore failures. *
             DropWatchPointAndUnlock(cx, wp, JSWP_LIVE);
-            DBG_LOCK(rt);
             if (rt->debuggerMutations != sample + 1)
                 next = (JSWatchPoint *)rt->watchPointList.next;
         }
     }
     DBG_UNLOCK(rt);
 }
 
 
@@ -620,17 +617,19 @@ js_watch_set(JSContext *cx, JSObject *ob
                             DBG_LOCK(rt);
                             DropWatchPointAndUnlock(cx, wp, JSWP_HELD);
                             return JS_FALSE;
                         }
                     }
 
                     argv[0] = OBJECT_TO_JSVAL(closure);
                     argv[1] = JSVAL_NULL;
-                    memset(argv + 2, 0, (nslots - 2) * sizeof(jsval));
+                    if(nslots>2){ 
+                   	 memset(argv + 2, 0, (nslots - 2) * sizeof(jsval));
+                    } 
 
                     memset(&frame, 0, sizeof(frame));
                     frame.script = script;
                     frame.regs = NULL;
                     if (script) {
                         JS_ASSERT(script->length >= JSOP_STOP_LENGTH);
                         regs.pc = script->code + script->length
                                   - JSOP_STOP_LENGTH;
@@ -842,20 +841,20 @@ JS_SetWatchPoint(JSContext *cx, JSObject
          */
         DBG_LOCK(rt);
         JS_ASSERT(!FindWatchPoint(rt, OBJ_SCOPE(obj), propid));
         JS_APPEND_LINK(&wp->links, &rt->watchPointList);
         ++rt->debuggerMutations;
     }
     wp->handler = handler;
     wp->closure = closure;
-    DBG_UNLOCK(rt);
 
 out:
     OBJ_DROP_PROPERTY(cx, obj, prop);
+    DBG_UNLOCK(rt);
     return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_ClearWatchPoint(JSContext *cx, JSObject *obj, jsval id,
                    JSWatchPointHandler *handlerp, void **closurep)
 {
     JSRuntime *rt;
@@ -866,17 +865,18 @@ JS_ClearWatchPoint(JSContext *cx, JSObje
     for (wp = (JSWatchPoint *)rt->watchPointList.next;
          &wp->links != &rt->watchPointList;
          wp = (JSWatchPoint *)wp->links.next) {
         if (wp->object == obj && SPROP_USERID(wp->sprop) == id) {
             if (handlerp)
                 *handlerp = wp->handler;
             if (closurep)
                 *closurep = wp->closure;
-            return DropWatchPointAndUnlock(cx, wp, JSWP_LIVE);
+    		DBG_UNLOCK(rt);
+		return DropWatchPointAndUnlock(cx, wp, JSWP_LIVE);
         }
     }
     DBG_UNLOCK(rt);
     if (handlerp)
         *handlerp = NULL;
     if (closurep)
         *closurep = NULL;
     return JS_TRUE;
@@ -894,17 +894,16 @@ JS_ClearWatchPointsForObject(JSContext *
     for (wp = (JSWatchPoint *)rt->watchPointList.next;
          &wp->links != &rt->watchPointList;
          wp = next) {
         next = (JSWatchPoint *)wp->links.next;
         if (wp->object == obj) {
             sample = rt->debuggerMutations;
             if (!DropWatchPointAndUnlock(cx, wp, JSWP_LIVE))
                 return JS_FALSE;
-            DBG_LOCK(rt);
             if (rt->debuggerMutations != sample + 1)
                 next = (JSWatchPoint *)rt->watchPointList.next;
         }
     }
     DBG_UNLOCK(rt);
     return JS_TRUE;
 }
 
@@ -919,17 +918,16 @@ JS_ClearAllWatchPoints(JSContext *cx)
     DBG_LOCK(rt);
     for (wp = (JSWatchPoint *)rt->watchPointList.next;
          &wp->links != &rt->watchPointList;
          wp = next) {
         next = (JSWatchPoint *)wp->links.next;
         sample = rt->debuggerMutations;
         if (!DropWatchPointAndUnlock(cx, wp, JSWP_LIVE))
             return JS_FALSE;
-        DBG_LOCK(rt);
         if (rt->debuggerMutations != sample + 1)
             next = (JSWatchPoint *)rt->watchPointList.next;
     }
     DBG_UNLOCK(rt);
     return JS_TRUE;
 }
 
 ************************************************************************

What could someone using Coverity possibly learn from reading the file they're patching?

To be a soldier in a regular army, instead of a monkey in a monkey suit, we should try to read the code we're breaking before we patch it.

     DBG_LOCK(rt);
     for (trap = (JSTrap *)rt->trapList.next;
...
             DestroyTrapAndUnlock(cx, trap);
-            DBG_LOCK(rt);
...
     }
     DBG_UNLOCK(rt);

For reference, here's the code the monkey didn't read, it was conveniently in the same file.

206 DestroyTrapAndUnlock(JSContext *cx, JSTrap *trap)
207 {
208     ++cx->runtime->debuggerMutations;
209     JS_REMOVE_LINK(&trap->links);
210     *trap->pc = (jsbytecode)trap->op;
211     DBG_UNLOCK(cx->runtime);