[webkit-reviews] review granted: [Bug 22876] Should centralize random number handling for JavaScriptCore & WebCore : [Attachment 26047] Updated patch for JavaScriptCore

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 22876: Should centralize random number handling for JavaScriptCore &
WebCore
https://bugs.webkit.org/show_bug.cgi?id=22876

Attachment 26047: Updated patch for JavaScriptCore
https://bugs.webkit.org/attachment.cgi?id=26047&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
A single patch for both JavaScriptCore and WebCore would be better.

There are two changes here. One is that the random number generator gets
initialized differently. The other is that the functions have new names and a
new header. I would have preferred to see those changes done independently so
we could separately discuss the merits of each.

I think it's a slightly tricky to initialize this correctly. As I understand
it, it's an error to call initializeRandomNumberGenerator at all when
JSC_MULTIPLE_THREADS  is off and also an error for anyone besides
initializeThreads to call it. I think it would be good to make it harder to use
wrong. For example, the definition in the header could be inside #if
ENABLE(JSC_MULTIPLE_THREADS), or the initialization function should be in its
own header -- people who are using the random number generator don't need to
know how the initialization works. And we could have some debugging code to
assert that it's called correctly. Generally we want functions that are hard to
"use wrong", and I think any calls to initializeRandomNumberGenerator are
likely to be wrong.

I'm not overjoyed with this, but I'll still say r=me