[webkit-reviews] review granted: [Bug 182605] Silence MAP_JIT warning for Network Process : [Attachment 342190] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Tadeu Zagallo <tzagallo at apple.com>'s
request for review:
Bug 182605: Silence MAP_JIT warning for Network Process
https://bugs.webkit.org/show_bug.cgi?id=182605

Attachment 342190: Patch

https://bugs.webkit.org/attachment.cgi?id=342190&action=review




--- Comment #7 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 342190
  --> https://bugs.webkit.org/attachment.cgi?id=342190
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342190&action=review

If you haven't already done so, please test this by adding a
RELEASE_ASSERT(processHasEntitlement("dynamic-codesigning")) in allowJIT(),
build for iOS, run it, and confirm that it doesn't crash.

r=me with fixes and testing.

> Source/JavaScriptCore/jit/ExecutableAllocator.cpp:122
> +#endif
> +    return true;

This "return true" should be in an #else section.  Otherwise, you'll have
unreachable code on PLATFORM(IOS).

> Source/WTF/wtf/cocoa/Entitlements.cpp:51
> +}

nit: add // namespace WTF

> Source/WTF/wtf/cocoa/Entitlements.h:32
> +}

nit: I would prefer that we add a "// namespace WTF" here.  This doesn't really
matter right now, but if more lines get added above to take this } further away
from the opening {, then the comment helps provide some context.  I'd just
prefer to do it all the time to make the pairing clear.