[webkit-reviews] review denied: [Bug 18661] Proxy server issue in Sunday's Nightly : [Attachment 20866] proposed fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 28 09:15:23 PDT 2008


Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 18661: Proxy server issue in Sunday's Nightly
http://bugs.webkit.org/show_bug.cgi?id=18661

Attachment 20866: proposed fix
http://bugs.webkit.org/attachment.cgi?id=20866&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
 192	     ExecState(JSGlobalObject*, JSObject* thisObject, JSObject*
globalThisValue, FunctionBodyNode*, ExecState* callingExecState, FunctionImp*,
const List& args) __attribute__ ((__always_inline__));

This needs to use the ALWAYS_INLINE macro so it will compile in non-GCC
compilers.

 66 extern HashTable arrayTable;
 67 extern HashTable dateTable;
 68 extern HashTable mathTable;
 69 extern HashTable numberTable;
 70 extern HashTable RegExpImpTable;
 71 extern HashTable RegExpObjectImpTable;
 72 extern HashTable stringTable;

Since these are global identifiers with external linkage, I would prefer that
they had more-specific suffixes on the names -- "Table" doesn't say it all to
me. But I guess these names aren't new.

6465	 class SyntaxErrorPrototype;
 66	struct ThreadClassInfoHashTables;
6567	 class TypeError;

We normally put the struct declarations in a separate paragraph below the class
ones.

 72	    if (classPropHashTableGetterFunction)
 73		return classPropHashTableGetterFunction(exec);
 74	    else
 75		return staticPropHashTable;

We normally omit else before return.

 134	     usleep(1000);

Is this portable to all platforms where we use pthreads?

r=me otherwise

review- because of the ALWAYS_INLINE issue above.


More information about the webkit-reviews mailing list