[webkit-reviews] review denied: [Bug 48571] [chromium] UUID generation does not work in Linux that has sandbox turned on : [Attachment 72268] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 28 17:53:10 PDT 2010


Dmitry Titov <dimich at chromium.org> has denied Jian Li <jianli at chromium.org>'s
request for review:
Bug 48571: [chromium] UUID generation does not work in Linux that has sandbox
turned on
https://bugs.webkit.org/show_bug.cgi?id=48571

Attachment 72268: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=72268&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72268&action=review

A few nits.
r- because  it implements it in the case where it should probably stay
unimplemented...

> WebCore/platform/UUID.cpp:99
>  #else

Is there a specific benefit in enabling it for everything else? Why not limit
to LINUX + CHROMIUM? If the bug is about Chromium Linux implementation, then it
should be narrowly scoped, otherwise there should be a note why specifically
the previously unimplemented functionality should be implemented.

> WebCore/platform/UUID.cpp:101
> +    for (size_t i = 0; i < sizeof(randomData) / sizeof(unsigned); ++i)

.. / sizeof(randomData[0]) ?

> WebCore/platform/UUID.cpp:113
> +    builder.append(String::format("%03x", randomData[2] >> 16));

seems it has to be (randomData[2]  >> 16 ) & 0x00000fff here...


More information about the webkit-reviews mailing list