[webkit-reviews] review granted: [Bug 21783] Need more granular control over allocation of executable memory : [Attachment 25808] Patch o' doom

[bugzilla-daemon at webkit.org]

Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 21783: Need more granular control over allocation of executable memory
https://bugs.webkit.org/show_bug.cgi?id=21783

Attachment 25808: Patch o' doom
https://bugs.webkit.org/attachment.cgi?id=25808&action=review

------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
> ExecutablePool::Allocation alloc = {(char*)mmap(NULL, n, PROT_READ |
PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0), n};

You don't need to cast the result of a void* memory allocation, but it seems to
be our style to do so. This should be a C++ style cast.

>class ExecutablePool : public RefCounted<ExecutablePool> {
>    struct Allocation {
>	 char* pages;
>	 size_t size;
>    };
>    typedef Vector<Allocation, 2> AllocationList;

Do we rely on implicit private: at the beginning of class definitions?

>	   if (n < (size_t)(m_end - m_freePtr)) {

Another C style cast that should be changed.

>	  if ((allocSize - n) > (size_t)(m_end - m_freePtr)) {

Another.

>      if ((((size_t)-1) - ExecutableAllocator::pageSize) <= request)

You should be using std::numeric_limits<size_t>::max() instead of casting -1 to
a size_t.

Other than that, r=me.