[webkit-reviews] review denied: [Bug 26611] Implement currentThreadStackBase on WinCE : [Attachment 31657] currentThreadStackBase for WinCE

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 22 11:24:20 PDT 2009


Adam Treat <treat at kde.org> has denied Joe Mason <joe.mason at torchmobile.com>'s
request for review:
Bug 26611: Implement currentThreadStackBase on WinCE
https://bugs.webkit.org/show_bug.cgi?id=26611

Attachment 31657: currentThreadStackBase for WinCE
https://bugs.webkit.org/attachment.cgi?id=31657&action=review

------- Additional Comments from Adam Treat <treat at kde.org>
> +2009-06-22  Yong Li	<yong.li at torchmobile.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=26611
> +	   Implement currentThreadStackBase on WINCE

You should include the full description of the change here.  As well as
document each new function you are adding where it is non-obvious.  Good to be
concise, but not at the expense of true explanation.

> +inline bool isPageWritable(void* p)

Please do not abbreviate the names of variables.  Spell out the variable using
a good descriptive name.  Err on code readability and maintenance instead of
less typing.

> +    void* sp = (void*)(&lower);
> +    register char* curPage = (char*)((DWORD)sp & ~(pageSize - 1));

Same as above.


More information about the webkit-reviews mailing list