[webkit-reviews] review granted: [Bug 235275] Do not use pas utils outside of libpas : [Attachment 449276] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 235275: Do not use pas utils outside of libpas
https://bugs.webkit.org/show_bug.cgi?id=235275

Attachment 449276: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 449276
  --> https://bugs.webkit.org/attachment.cgi?id=449276
Patch

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

> Source/WebCore/ChangeLog:8
> +	   We should not use any utility functions from libpas outside of
bmalloc.

Can we change how the headers are structured so the compile will fail too? That
way the rule would “guard itself” and we would not need vigilant programmers
thinking about this rule. Typically in separate frameworks the technique would
be “mark header internal”; headers would not be installed. Is there a similar
technique at least for the bmalloc/WebCore boundary if not bmalloc/WTF?

> Source/WTF/wtf/MathExtras.h:773
> +inline uint64_t reverseBits64(uint64_t value)

Why add this unused and untested function?

> Source/WTF/wtf/MathExtras.h:782
> +    return ((uint64_t)reverseBits32((unsigned)value) << (uint64_t)32)

I suggest uint32_t here instead of unsigned

> Source/WTF/wtf/MathExtras.h:783
> +	   | (uint64_t)reverseBits32((unsigned)(value >> (uint64_t)32));

Ditto