Monkey see monkey do? Monkeys who don't know C shouldn't do Coverity.

diff -r 8c01ebb8179d js/src/jsobj.cpp
--- a/js/src/jsobj.cpp	Tue Jun 16 16:00:23 2009 +0300
+++ b/js/src/jsobj.cpp	Wed Jun 17 11:16:25 2009 +0300
@@ -1198,17 +1198,19 @@ js_ComputeFilename(JSContext *cx, JSStac
     flags = JS_GetScriptFilenameFlags(caller->script);
     if ((flags & JSFILENAME_PROTECTED) &&
         principals &&
         strcmp(principals->codebase, "[System Principal]")) {
         *linenop = 0;
         return principals->codebase;
     }
 
-    jsbytecode *pc = caller->regs->pc;
+    if (caller->regs)
+    	jsbytecode *pc = caller->regs->pc;
+
     if (caller->regs && js_GetOpcode(cx, caller->script, pc) == JSOP_EVAL) {
         JS_ASSERT(js_GetOpcode(cx, caller->script, pc + JSOP_EVAL_LENGTH) == JSOP_LINENO);
         *linenop = GET_UINT16(pc + JSOP_EVAL_LENGTH);
     } else {
         *linenop = js_FramePCToLineNumber(cx, caller);
     }
     return caller->script->filename;
 }

What happened?

Someone insisted that a tester or person (hereafter "monkey") run Coverity and fix the warnings it reported. The specific complaint from Coverity is that caller->regs was dereferenced directly first and then null checked later.

What was wrong with the patch?

Unfortunately, the monkey here didn't pay attention to how scope works, or think much at all.

Specifically, the newly added if creates a new local scope for |jsbytecode *pc|, and that scope goes away before the next line which tries to use |pc|. Since the defintion of |pc| is gone, the patched code doesn't compile.

Was there sort of a problem?

Yes, but asking a monkey to fix it, or asking a senior engineer to review changes which on average (5/5) are wrong is not a good use of anyone's time.

What should be done?

1. If managers demand that monkeys do this, let them try 5 changes
2. Review them, show management that they failed (if you're lucky 5/5 times)
3. Explain that your senior engineers can use Coverity when they have time to look for problems that might be worth fixing

How should that specific bug be fixed?

You can see how the bug was fixed correctly in bug 449897 reverse INULL in js_ComputeFilename.